<div dir="ltr"><div class="gmail_quote"><div dir="ltr">On Tue, May 1, 2018 at 7:50 AM Wei Mi via llvm-commits <<a href="mailto:llvm-commits@lists.llvm.org">llvm-commits@lists.llvm.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Author: wmi<br>
Date: Tue May 1 07:47:24 2018<br>
New Revision: 331266<br>
<br>
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=331266&view=rev" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project?rev=331266&view=rev</a><br>
Log:<br>
Fix the issue that ComputeValueKnownInPredecessors only handles the case when<br>
phi is on lhs of a comparison op.<br>
<br>
For the following testcase,<br>
L1:<br>
<br>
%t0 = add i32 %m, 7<br>
%t3 = icmp eq i32* %t2, null<br>
br i1 %t3, label %L3, label %L2<br>
<br>
L2:<br>
<br>
%t4 = load i32, i32* %t2, align 4<br>
br label %L3<br>
<br>
L3:<br>
<br>
%t5 = phi i32 [ %t0, %L1 ], [ %t4, %L2 ]<br>
%t6 = icmp eq i32 %t0, %t5<br>
br i1 %t6, label %L4, label %L5<br>
<br>
We know if we go through the path L1 --> L3, %t6 should always be true. However<br>
currently, if the rhs of the eq comparison is phi, JumpThreading fails to<br>
evaluate %t6 to true. And we know that Instcombine cannot guarantee always<br>
canonicalizing phi to the left hand side of the comparison operation according<br>
to the operand priority comparison mechanism in instcombine. The patch handles<br>
the case when rhs of the comparison op is a phi.<br>
<br>
Differential Revision: <a href="https://reviews.llvm.org/D46275" rel="noreferrer" target="_blank">https://reviews.llvm.org/D46275</a><br>
<br>
<br>
Modified:<br>
llvm/trunk/lib/Transforms/Scalar/JumpThreading.cpp<br>
llvm/trunk/test/Other/pr32085.ll<br>
llvm/trunk/test/Transforms/JumpThreading/phi-known.ll<br>
<br>
Modified: llvm/trunk/lib/Transforms/Scalar/JumpThreading.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Scalar/JumpThreading.cpp?rev=331266&r1=331265&r2=331266&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Scalar/JumpThreading.cpp?rev=331266&r1=331265&r2=331266&view=diff</a><br>
==============================================================================<br>
--- llvm/trunk/lib/Transforms/Scalar/JumpThreading.cpp (original)<br>
+++ llvm/trunk/lib/Transforms/Scalar/JumpThreading.cpp Tue May 1 07:47:24 2018<br>
@@ -752,6 +752,8 @@ bool JumpThreadingPass::ComputeValueKnow<br>
CmpInst::Predicate Pred = Cmp->getPredicate();<br>
<br>
PHINode *PN = dyn_cast<PHINode>(CmpLHS);<br>
+ if (!PN)<br>
+ PN = dyn_cast<PHINode>(CmpRHS);<br>
if (PN && PN->getParent() == BB) {<br>
const DataLayout &DL = PN->getModule()->getDataLayout();<br>
// We can do this simplification if any comparisons fold to true or false.<br>
@@ -762,14 +764,24 @@ bool JumpThreadingPass::ComputeValueKnow<br>
LVI->enableDT();<br>
for (unsigned i = 0, e = PN->getNumIncomingValues(); i != e; ++i) {<br>
BasicBlock *PredBB = PN->getIncomingBlock(i);<br>
- Value *LHS = PN->getIncomingValue(i);<br>
- Value *RHS = CmpRHS->DoPHITranslation(BB, PredBB);<br>
-<br>
+ Value *LHS, *RHS;<br>
+ if (PN == CmpLHS) {<br>
+ LHS = PN->getIncomingValue(i);<br>
+ RHS = CmpRHS->DoPHITranslation(BB, PredBB);<br>
+ } else {<br>
+ LHS = CmpLHS->DoPHITranslation(BB, PredBB);<br>
+ RHS = PN->getIncomingValue(i);<br>
+ }<br>
Value *Res = SimplifyCmpInst(Pred, LHS, RHS, {DL});<br>
if (!Res) {<br>
if (!isa<Constant>(RHS))<br>
continue;<br>
<br>
+ // getPredicateOnEdge call will make no sense if LHS is defined in BB.<br>
+ auto LHSInst = dyn_cast<Instruction>(LHS);<br>
+ if (LHSInst && LHSInst->getParent() == BB)<br>
+ continue;<br>
+<br>
LazyValueInfo::Tristate<br>
ResT = LVI->getPredicateOnEdge(Pred, LHS,<br>
cast<Constant>(RHS), PredBB, BB,<br>
<br>
Modified: llvm/trunk/test/Other/pr32085.ll<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Other/pr32085.ll?rev=331266&r1=331265&r2=331266&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Other/pr32085.ll?rev=331266&r1=331265&r2=331266&view=diff</a><br>
==============================================================================<br>
--- llvm/trunk/test/Other/pr32085.ll (original)<br>
+++ llvm/trunk/test/Other/pr32085.ll Tue May 1 07:47:24 2018<br>
@@ -1,6 +1,7 @@<br>
; RUN: opt -S -O1 < %s -o %t1.ll<br>
;; Show that there's no difference after running another simplify CFG<br>
-; RUN: opt -S -simplifycfg < %t1.ll -o %t2.ll<br>
+; RUN: opt -S -simplifycfg < %t1.ll -o %t2.ll <br>
+; RUN: sed -i 's/; preds = .*//g' %t1.ll %t2.ll<br></blockquote><div><br></div><div>Rather than using 'sed' with its portability problems, just rinse it through a no-op opt run:</div><div>; RUN: opt -S < %t1.ll -o %t2.ll</div><div>; RUN: opt -S -simplifycfg < %t1.ll -o %t3.ll</div><div>; RUN: diff %t2.ll %t3.ll</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
; RUN: diff %t1.ll %t2.ll<br>
<br>
; Test from LoopSink pass, leaves some single-entry single-exit basic blocks.<br>
<br>
Modified: llvm/trunk/test/Transforms/JumpThreading/phi-known.ll<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/JumpThreading/phi-known.ll?rev=331266&r1=331265&r2=331266&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/JumpThreading/phi-known.ll?rev=331266&r1=331265&r2=331266&view=diff</a><br>
==============================================================================<br>
--- llvm/trunk/test/Transforms/JumpThreading/phi-known.ll (original)<br>
+++ llvm/trunk/test/Transforms/JumpThreading/phi-known.ll Tue May 1 07:47:24 2018<br>
@@ -64,3 +64,41 @@ exit:<br>
ret void<br>
}<br>
<br>
+; The eq predicate is always true if we go through the path from<br>
+; L1 to L3, no matter the phi result %t5 is on the lhs or rhs of<br>
+; the predicate.<br>
+declare void @goo()<br>
+declare void @hoo()<br>
+<br>
+define void @test3(i32 %m, i32** %t1) {<br>
+L1:<br>
+ %t0 = add i32 %m, 7<br>
+ %t2 = load i32*, i32** %t1, align 8<br>
+; CHECK-LABEL: @test3<br>
+; CHECK: %t3 = icmp eq i32* %t2, null<br>
+; CHECK: br i1 %t3, label %[[LABEL2:.*]], label %[[LABEL1:.*]]<br>
+<br>
+ %t3 = icmp eq i32* %t2, null<br>
+ br i1 %t3, label %L3, label %L2<br>
+<br>
+; CHECK: [[LABEL1]]:<br>
+; CHECK-NEXT: %t4 = load i32, i32* %t2, align 4<br>
+L2:<br>
+ %t4 = load i32, i32* %t2, align 4<br>
+ br label %L3<br>
+<br>
+L3:<br>
+ %t5 = phi i32 [ %t0, %L1 ], [ %t4, %L2 ]<br>
+ %t6 = icmp eq i32 %t0, %t5<br>
+ br i1 %t6, label %L4, label %L5<br>
+<br>
+; CHECK: [[LABEL2]]:<br>
+; CHECK-NEXT: call void @goo()<br>
+L4:<br>
+ call void @goo()<br>
+ ret void<br>
+<br>
+L5:<br>
+ call void @hoo()<br>
+ ret void<br>
+}<br>
<br>
<br>
_______________________________________________<br>
llvm-commits mailing list<br>
<a href="mailto:llvm-commits@lists.llvm.org" target="_blank">llvm-commits@lists.llvm.org</a><br>
<a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits</a><br>
</blockquote></div></div>