Skip to content

S
swiftshader
Commit bbd449d2 by Jim Stichnoth
Options

Subzero: Fix a bug in advanced phi lowering.

The motivating example (simplified) is this: %__698:eax = phi i32 [ %__646:ebx, %__188 ] // LIVEEND={%__646:ebx} %__617:bh = phi i8 [ %__618:ah, %__188 ] // LIVEEND={%__618:ah} %__615:bl = phi i8 [ %__616:al, %__188 ] // LIVEEND={%__616:al} By default, it first lowers the __698 assignment. However, that assignment has two "predecessors" because there are two other instructions whose dest variable aliases the __698 assignment's source operand. This triggers an assertion failure where we assume there is only one predecessor. The fix is two-pronged. First, we go ahead and generate as many temp assignments as needed to break the cycle, simply by changing an "if" to a "while". Second, when we need to break a cycle, we give preference to an instruction with only one predecessor so that only one temp assignment needs to be added. (It might be possible to prove that the second approach, i.e. preferring single-predecessor assignments, makes the first approach unnecessary, i.e. changing "if" to "while".) This change has no effect on the x86 output for spec2k. BUG= https://bugs.chromium.org/p/nativeclient/issues/detail?id=4365 R=jpp@chromium.org Review URL: https://codereview.chromium.org/1839263003 .
parent 467ffe51
Showing with 20 additions and 10 deletions
src/IceCfgNode.cpp
...@@ -325,11 +325,15 @@ public: ...@@ -325,11 +325,15 @@ public:
using PhiDescList = llvm::SmallVector<PhiDesc, 32>; using PhiDescList = llvm::SmallVector<PhiDesc, 32>;
// Always pick NumPred=0 over NumPred>0. // Always pick NumPred=0 over NumPred>0.
constexpr int32_t WeightNoPreds = 4; constexpr int32_t WeightNoPreds = 8;
// Prefer Src as a register because the register might free up. // Prefer Src as a register because the register might free up.
constexpr int32_t WeightSrcIsReg = 2; constexpr int32_t WeightSrcIsReg = 4;
// Prefer Dest not as a register because the register stays free longer. // Prefer Dest not as a register because the register stays free longer.
constexpr int32_t WeightDestNotReg = 1; constexpr int32_t WeightDestNotReg = 2;
// Prefer NumPred=1 over NumPred>1. This is used as a tiebreaker when a
// dependency cycle must be broken so that hopefully only one temporary
// assignment has to be added to break the cycle.
constexpr int32_t WeightOnePred = 1;
bool sameVarOrReg(TargetLowering *Target, const Variable *Var1, bool sameVarOrReg(TargetLowering *Target, const Variable *Var1,
const Operand *Opnd) { const Operand *Opnd) {
...@@ -357,12 +361,18 @@ bool sameVarOrReg(TargetLowering *Target, const Variable *Var1, ...@@ -357,12 +361,18 @@ bool sameVarOrReg(TargetLowering *Target, const Variable *Var1,
} }
// Update NumPred for all Phi assignments using Var as their Dest variable. // Update NumPred for all Phi assignments using Var as their Dest variable.
// Also update Weight if NumPred dropped from 1 to 0. // Also update Weight if NumPred dropped from 2 to 1, or 1 to 0.
void updatePreds(PhiDescList &Desc, TargetLowering *Target, Variable *Var) { void updatePreds(PhiDescList &Desc, TargetLowering *Target, Variable *Var) {
for (PhiDesc &Item : Desc) { for (PhiDesc &Item : Desc) {
if (!Item.Processed && sameVarOrReg(Target, Var, Item.Dest)) { if (!Item.Processed && sameVarOrReg(Target, Var, Item.Dest)) {
if (--Item.NumPred == 0) { --Item.NumPred;
Item.Weight += WeightNoPreds; if (Item.NumPred == 1) {
// If NumPred changed from 2 to 1, add in WeightOnePred.
Item.Weight += WeightOnePred;
} else if (Item.NumPred == 0) {
// If NumPred changed from 1 to 0, subtract WeightOnePred and add in
// WeightNoPreds.
Item.Weight += (WeightNoPreds - WeightOnePred);
} }
} }
} }
...@@ -479,6 +489,8 @@ void CfgNode::advancedPhiLowering() { ...@@ -479,6 +489,8 @@ void CfgNode::advancedPhiLowering() {
int32_t Weight = 0; int32_t Weight = 0;
if (Item.NumPred == 0) if (Item.NumPred == 0)
Weight += WeightNoPreds; Weight += WeightNoPreds;
if (Item.NumPred == 1)
Weight += WeightOnePred;
if (auto *Var = llvm::dyn_cast<Variable>(Item.Src)) if (auto *Var = llvm::dyn_cast<Variable>(Item.Src))
if (Var->hasReg()) if (Var->hasReg())
Weight += WeightSrcIsReg; Weight += WeightSrcIsReg;
...@@ -498,20 +510,18 @@ void CfgNode::advancedPhiLowering() { ...@@ -498,20 +510,18 @@ void CfgNode::advancedPhiLowering() {
for (PhiDesc &Item : Desc) { for (PhiDesc &Item : Desc) {
if (Item.Processed) if (Item.Processed)
continue; continue;
int32_t Weight = 0; const int32_t Weight = Item.Weight;
Weight = Item.Weight;
if (Weight > BestWeight) { if (Weight > BestWeight) {
BestItem = &Item; BestItem = &Item;
BestWeight = Weight; BestWeight = Weight;
} }
} }
assert(BestWeight >= 0); assert(BestWeight >= 0);
assert(BestItem->NumPred <= 1);
Variable *Dest = BestItem->Dest; Variable *Dest = BestItem->Dest;
Operand *Src = BestItem->Src; Operand *Src = BestItem->Src;
assert(!sameVarOrReg(Target, Dest, Src)); assert(!sameVarOrReg(Target, Dest, Src));
// Break a cycle by introducing a temporary. // Break a cycle by introducing a temporary.
if (BestItem->NumPred) { while (BestItem->NumPred > 0) {
bool Found = false; bool Found = false;
// If the target instruction "A=B" is part of a cycle, find the "X=A" // If the target instruction "A=B" is part of a cycle, find the "X=A"
// assignment in the cycle because it will have to be rewritten as // assignment in the cycle because it will have to be rewritten as
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment