Differential D101273
[LazyValueInfo] Insert an Overdefined placeholder to prevent infinite recursion Authored by guopeilin on Apr 25 2021, 7:27 PM.
Details getValueFromCondition() uses a Visited set to record the intermediate value. "%tmp3 = or i1 undef, %tmp4" "%tmp4 = or i1 undef, %tmp3" To prevent this, we can insert an Overdefined placeholder into the set
Diff Detail Event TimelineThis comment was removed by guopeilin. Comment Actions The bug is reported in https://bugs.llvm.org/show_bug.cgi?id=50119  This bug is generated in the following logic: Firstly, we can see that this function has a loop that contains the two red basic blocks. And this loop's preheader is for.body.i.us.1.preheader. After we ProcessBlock, the preheader becomes dead, then we decide to DeleteDeadBlock. And within the functions DeleteDeadBlock, we visit the dead block's successors and remove the successors from its predecessor. That is we just detach the dead block and the unreachable blocks still remain within the function. What's more, for those successors, we also try to simplify their phi nodes. In this case, the loop that has two red blocks, its loop header for.body.i.us.1 has a PHI node, which has two incoming values, one from the preheader(predecessor), the other one from the loop exit. Now that we have removed the header from its predecessor, we can remove the incoming value from its predecessor and replace the phi node use with another incoming value. After the DeleteDeadBlock, the loop might become this: for.body.i.us.1: ; preds = %lor.end.i.us.1 br i1 %spec.select44.i.us.1, label %lor.end.i.us.1, label %lor.rhs.i.us.1 lor.rhs.i.us.1: ; preds = %for.body.i.us.1 %conv5.i.us.1 = trunc i32 undef to i8 br label %lor.end.i.us.1 lor.end.i.us.1: ; preds = %lor.rhs.i.us.1, %for.body.i.us.1 %.b31.i.us.1 = or i1 undef, %spec.select44.i.us.1 %spec.select44.i.us.1 = or i1 undef, %.b31.i.us.1 br i1 undef, label %for.end18.loopexit42.i.1.loopexit, label %for.body.i.us.1 So in the block lor.end.i.us.1, there exists some invalid IRs that the use not dominated by its def in a self-loop. Comment Actions Right now this patch set the argument KeepOneInputPHIs of function DeleteDeadBlocks to be true, that is we only allow replacing the phi node with its incoming value if this incoming value is a constant Comment Actions I *think* that the current behavior is ok. Dominance is not defined in unreachable code, so cyclic references etc are allowed (by the verifier etc). What is the problem that this is causing? Comment Actions https://bugs.llvm.org/show_bug.cgi?id=50119 BinaryOperator *BO = dyn_cast<BinaryOperator>(Cond);
Value *BL = BO->getOperand(0);
Value *BR = BO->getOperand(1);
if (BL == Cond || BR == Cond)
return ValueLatticeElement::getOverdefined();
return intersect(getValueFromCondition(Val, BL, isTrueDest, Visited),
getValueFromCondition(Val, BR, isTrueDest, Visited));At some time, let's consume that the condition is %spec.select44.i.us.1 = or i1 undef, %.b31.i.us.1, its second operand is %.b31.i.us.1. Comment Actions @guopeilin getValueFromCondition() uses a Visited set that should be used to prevent this. You need to insert an Overdefined placeholder into the set before computing the actual value.
Comment Actions LGTM
| ||||||||||||||||||
It should be possible to drop this code now.