[PATCH] D46275: [JumpThreading] ComputeValueKnownInPredecessors only handles the case when phi is on lhs of a comparison op

Wei Mi via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Apr 30 10:41:02 PDT 2018


wmi created this revision.
wmi added reviewers: davidxl, danielcdh.

For the following testcase, 
https://reviews.llvm.org/L1:

  %t0 = add i32 %m, 7
  %t3 = icmp eq i32* %t2, null
  br i1 %t3, label %L3, label %L2

L2:

  %t4 = load i32, i32* %t2, align 4
  br label %L3

L3:

  %t5 = phi i32 [ %t0, %L1 ], [ %t4, %L2 ]
  %t6 = icmp eq i32 %t0, %t5
  br i1 %t6, label %L4, label %L5

We know if we go through the path https://reviews.llvm.org/L1 --> L3, %t6 should always be true. However, currently, if the rhs of the eq comparison is phi, JumpThreading fails to evaluate %t6 to true. And we know that Instcombine cannot guarantee always canonicalizing phi to the left hand side of the comparison operation according to the operand priority comparison mechanism in instcombine.

The patch handles the case when rhs of the comparison op is a phi.


Repository:
  rL LLVM

https://reviews.llvm.org/D46275

Files:
  lib/Transforms/Scalar/JumpThreading.cpp
  test/Other/pr32085.ll
  test/Transforms/JumpThreading/phi-known.ll


Index: test/Transforms/JumpThreading/phi-known.ll
===================================================================
--- test/Transforms/JumpThreading/phi-known.ll
+++ test/Transforms/JumpThreading/phi-known.ll
@@ -64,3 +64,41 @@
   ret void
 }
 
+; The eq predicate is always true if we go through the path from
+; L1 to L3, no matter the phi result %t5 is on the lhs or rhs of
+; the predicate.
+declare void @goo()
+declare void @hoo()
+
+define void @test3(i32 %m, i32** %t1) {
+L1:
+  %t0 = add i32 %m, 7
+  %t2 = load i32*, i32** %t1, align 8
+; CHECK-LABEL: @test3
+; CHECK: %t3 = icmp eq i32* %t2, null
+; CHECK: br i1 %t3, label %[[LABEL2:.*]], label %[[LABEL1:.*]]
+
+  %t3 = icmp eq i32* %t2, null
+  br i1 %t3, label %L3, label %L2
+
+; CHECK: [[LABEL1]]:
+; CHECK-NEXT: %t4 = load i32, i32* %t2, align 4
+L2:
+  %t4 = load i32, i32* %t2, align 4
+  br label %L3
+
+L3:
+  %t5 = phi i32 [ %t0, %L1 ], [ %t4, %L2 ]
+  %t6 = icmp eq i32 %t0, %t5
+  br i1 %t6, label %L4, label %L5
+
+; CHECK: [[LABEL2]]:
+; CHECK-NEXT: call void @goo()
+L4:
+  call void @goo()
+  ret void
+
+L5:
+  call void @hoo()
+  ret void
+}
Index: test/Other/pr32085.ll
===================================================================
--- test/Other/pr32085.ll
+++ test/Other/pr32085.ll
@@ -1,6 +1,7 @@
 ; RUN: opt -S -O1 < %s -o %t1.ll
 ;; Show that there's no difference after running another simplify CFG
-; RUN: opt -S -simplifycfg < %t1.ll -o %t2.ll
+; RUN: opt -S -simplifycfg < %t1.ll -o %t2.ll 
+; RUN: sed -i 's/; preds = .*//g' %t1.ll %t2.ll
 ; RUN: diff %t1.ll %t2.ll
 
 ; Test from LoopSink pass, leaves some single-entry single-exit basic blocks.
Index: lib/Transforms/Scalar/JumpThreading.cpp
===================================================================
--- lib/Transforms/Scalar/JumpThreading.cpp
+++ lib/Transforms/Scalar/JumpThreading.cpp
@@ -752,6 +752,8 @@
     CmpInst::Predicate Pred = Cmp->getPredicate();
 
     PHINode *PN = dyn_cast<PHINode>(CmpLHS);
+    if (!PN)
+      PN = dyn_cast<PHINode>(CmpRHS);
     if (PN && PN->getParent() == BB) {
       const DataLayout &DL = PN->getModule()->getDataLayout();
       // We can do this simplification if any comparisons fold to true or false.
@@ -762,9 +764,14 @@
         LVI->enableDT();
       for (unsigned i = 0, e = PN->getNumIncomingValues(); i != e; ++i) {
         BasicBlock *PredBB = PN->getIncomingBlock(i);
-        Value *LHS = PN->getIncomingValue(i);
-        Value *RHS = CmpRHS->DoPHITranslation(BB, PredBB);
-
+        Value *LHS, *RHS;
+        if (PN == CmpLHS) {
+          LHS = PN->getIncomingValue(i);
+          RHS = CmpRHS->DoPHITranslation(BB, PredBB);
+        } else {
+          LHS = CmpLHS->DoPHITranslation(BB, PredBB);
+          RHS = PN->getIncomingValue(i);
+        }
         Value *Res = SimplifyCmpInst(Pred, LHS, RHS, {DL});
         if (!Res) {
           if (!isa<Constant>(RHS))


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D46275.144584.patch
Type: text/x-patch
Size: 2894 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20180430/7e601ffc/attachment.bin>


More information about the llvm-commits mailing list