[PATCH] D34135: [JumpThreading] Use DT to avoid processing dead blocks
Brian Rzycki via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Tue Mar 6 14:18:36 PST 2018
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
More information about the llvm-commits
mailing list