[llvm] r335675 - [JumpThreading] Don't try to rewrite a use if it's already valid.

[Michael Zolotukhin via llvm-commits]

Author: mzolotukhin
Date: Tue Jun 26 15:19:48 2018
New Revision: 335675

URL: http://llvm.org/viewvc/llvm-project?rev=335675&view=rev
Log:
[JumpThreading] Don't try to rewrite a use if it's already valid.

Summary:
When recording uses we need to rewrite after cloning a loop we need to
check if the use is not dominated by the original def. The initial
assumption was that the cloned basic block will introduce a new path and
thus the original def will only dominate the use if they are in the same
BB, but as the reproducer from PR37745 shows it's not always the case.

This fixes PR37745.

Reviewers: haicheng, Ka-Ka

Subscribers: hiraditya, llvm-commits

Differential Revision: https://reviews.llvm.org/D48111

Added:
    llvm/trunk/test/Transforms/JumpThreading/PR37745.ll
Modified:
    llvm/trunk/lib/Transforms/Scalar/JumpThreading.cpp

Modified: llvm/trunk/lib/Transforms/Scalar/JumpThreading.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Scalar/JumpThreading.cpp?rev=335675&r1=335674&r2=335675&view=diff
==============================================================================
--- llvm/trunk/lib/Transforms/Scalar/JumpThreading.cpp (original)
+++ llvm/trunk/lib/Transforms/Scalar/JumpThreading.cpp Tue Jun 26 15:19:48 2018
@@ -2009,6 +2009,14 @@ bool JumpThreadingPass::ThreadEdge(Basic
       PredTerm->setSuccessor(i, NewBB);
     }
 
+  // Enqueue required DT updates.
+  DDT->applyUpdates({{DominatorTree::Insert, NewBB, SuccBB},
+                     {DominatorTree::Insert, PredBB, NewBB},
+                     {DominatorTree::Delete, PredBB, BB}});
+
+  // Apply all updates we queued with DDT and get the updated Dominator Tree.
+  DominatorTree *DT = &DDT->flush();
+
   // If there were values defined in BB that are used outside the block, then we
   // now have to update all uses of the value to use either the original value,
   // the cloned value, or some PHI derived value.  This can require arbitrary
@@ -2018,16 +2026,16 @@ bool JumpThreadingPass::ThreadEdge(Basic
   for (Instruction &I : *BB) {
     SmallVector<Use*, 16> UsesToRename;
 
-    // Scan all uses of this instruction to see if it is used outside of its
-    // block, and if so, record them in UsesToRename. Also, skip phi operands
-    // from PredBB - we'll remove them anyway.
+    // Scan all uses of this instruction to see if their uses are no longer
+    // dominated by the previous def and if so, record them in UsesToRename.
+    // Also, skip phi operands from PredBB - we'll remove them anyway.
     for (Use &U : I.uses()) {
       Instruction *User = cast<Instruction>(U.getUser());
       if (PHINode *UserPN = dyn_cast<PHINode>(User)) {
         if (UserPN->getIncomingBlock(U) == BB ||
             UserPN->getIncomingBlock(U) == PredBB)
           continue;
-      } else if (User->getParent() == BB)
+      } else if (DT->dominates(&I, U))
         continue;
 
       UsesToRename.push_back(&U);
@@ -2046,12 +2054,6 @@ bool JumpThreadingPass::ThreadEdge(Basic
       SSAUpdate.AddUse(VarNum, U);
   }
 
-  DDT->applyUpdates({{DominatorTree::Insert, NewBB, SuccBB},
-                     {DominatorTree::Insert, PredBB, NewBB},
-                     {DominatorTree::Delete, PredBB, BB}});
-
-  // Apply all updates we queued with DDT and get the updated Dominator Tree.
-  DominatorTree *DT = &DDT->flush();
   SSAUpdate.RewriteAllUses(DT);
 
   // At this point, the IR is fully up to date and consistent.  Do a quick scan

Added: llvm/trunk/test/Transforms/JumpThreading/PR37745.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/JumpThreading/PR37745.ll?rev=335675&view=auto
==============================================================================
--- llvm/trunk/test/Transforms/JumpThreading/PR37745.ll (added)
+++ llvm/trunk/test/Transforms/JumpThreading/PR37745.ll Tue Jun 26 15:19:48 2018
@@ -0,0 +1,19 @@
+; RUN: opt -jump-threading -verify-each -S -mtriple=x86_64-- -o - %s
+
+define void @foo() {
+entry:
+  br i1 false, label %A, label %B
+
+A:
+  %x = phi i32 [ undef, %entry ], [ %z, %B ]
+  br label %B
+
+B:
+  %y = phi i32 [ undef, %entry ], [ %x, %A ]
+  %z = add i32 %y, 1
+  %cmp = icmp ne i32 %z, 0
+  br i1 %cmp, label %exit, label %A
+
+exit:
+  ret void
+}