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

[Brian Rzycki via Phabricator via llvm-commits]

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?

This patch passes ninja check-all and test-suite builds on x86_64.

@uabelho , the test case needs one small change to the FileCheck verification block:

  ; The
  ;  br i1 undef, label %bb6, label %bb1
  ; is replaced by
  ;  br label %bb6
  ; and the rest of the function is unreachable from entry and jump-threading
  ; shouldn't even ask LazyValueInfo about things in the dead blocks.
  
  ; CHECK:      define void @f(i32 %p1) {
  ; CHECK-NEXT: bb6:
  ; CHECK-NEXT:   ret void

For whatever reason JumpThreading now removes `%0 = icmp eq i32 %p1, 0` from `bb6:`.


https://reviews.llvm.org/D34135