[PATCH] D34135: [JumpThreading] Use DT to avoid processing dead blocks

[Daniel Berlin via llvm-commits]

On Tue, Mar 6, 2018 at 2:18 PM, Brian Rzycki via Phabricator <
reviews at reviews.llvm.org> wrote:

> brzycki added a comment.
>
> I have analyzed this failure and have what I believe (in @dberlin 's
> words) to be the smallest hammer fix:
>
>   $ git diff lib/Analysis/LazyValueInfo.cpp
>   diff --git a/llvm/lib/Analysis/LazyValueInfo.cpp b/llvm/lib/Analysis/
> LazyValueInfo.cpp
>   index 65fd007dc0b..36e66f7f6ef 100644
>   --- a/llvm/lib/Analysis/LazyValueInfo.cpp
>   +++ b/llvm/lib/Analysis/LazyValueInfo.cpp
>   @@ -1145,9 +1145,17 @@ getValueFromConditionImpl(Value *Val, Value
> *Cond, bool isTrueDest,
>                 (!isTrueDest && BO->getOpcode() != BinaryOperator::Or))
>        return ValueLatticeElement::getOverdefined();
>
>   -  auto RHS = getValueFromCondition(Val, BO->getOperand(0), isTrueDest,
> Visited);
>   -  auto LHS = getValueFromCondition(Val, BO->getOperand(1), isTrueDest,
> Visited);
>   -  return intersect(RHS, LHS);
>   +  // Prevent infinite recursion if Cond references itself as in this
> example:
>   +  //  Cond: "%tmp4 = and i1 %tmp4, undef"
>   +  //    BL: "%tmp4 = and i1 %tmp4, undef"
>   +  //    BR: "i1 undef"
>   +  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));
>    }
>
> The infinite recursion happens because `Visited` is not altered or updated
> until `getValueFromConditionImpl()` returns a value. In this case
> `getValueFromConditionImpl()` calls `getValueFromCondition()` which calls
> back to `getValueFromConditionImpl()` in an attempt to resolve the LHS and
> RHS of a valid `BinaryOperator`.
>
> In our case this specific instruction we have `%tmp4 = and i1 %tmp4,
> undef` which I don't quite understand. Shouldn't this be invalid SSA form?
>


We allow somewhat illegal IR in unreachable blocks.
There are some mailing list discussions around this.

This is under the guise of it  being easier than knowing when the block is
dead and removing it.
In my experience, it generates more bugs trying to avoid illegal IR than it
is worth, and most passes end up trying to find a way to at least avoid
visiting dead blocks.
(Some try to be resilient and visit dead blocks, but they rarely survive it
over time, and you will see a whole host of bugzilla bugs in this category
:P)

Handling this is much easier in non-lazy passes.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20180306/6c55e4c4/attachment.html>