[PATCH] D60284: [JumpThreading] [PR40992] Fix miscompile when folding a successor of an indirectbr

[Brian Rzycki via Phabricator via llvm-commits]

brzycki updated this revision to Diff 193781.
brzycki added a comment.

Address review comments.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D60284/new/

https://reviews.llvm.org/D60284

Files:
  llvm/lib/Transforms/Scalar/JumpThreading.cpp
  llvm/test/Transforms/JumpThreading/pr40992-indirectbr-folding.ll


Index: llvm/test/Transforms/JumpThreading/pr40992-indirectbr-folding.ll
===================================================================
--- /dev/null
+++ llvm/test/Transforms/JumpThreading/pr40992-indirectbr-folding.ll
@@ -0,0 +1,42 @@
+; RUN: opt -S < %s -jump-threading | FileCheck %s
+
+; PR40992: Do not incorrectly fold %bb5 into an unconditional br to %bb7.
+;          Also verify we correctly thread %bb1 -> %bb7 when %c is false.
+
+define i32 @jtbr(i1 %v1, i1 %v2, i1 %v3) {
+; CHECK: bb0:
+bb0:
+  br label %bb1
+
+; CHECK: bb1:
+bb1:
+  %c = and i1 %v1, %v2
+  br i1 %c, label %bb2, label %bb5
+
+; CHECK: bb2:
+; CHECK-NEXT: select
+; CHECK-NEXT: indirectbr i8* %ba, [label %bb3, label %bb5]
+bb2:
+  %ba = select i1 %v3, i8* blockaddress(@jtbr, %bb3), i8* blockaddress(@jtbr, %bb4)
+  indirectbr i8* %ba, [label %bb3, label %bb4]
+
+; CHECK: bb3:
+bb3:
+  br label %bb1
+
+; CHECK-NOT: bb4:
+bb4:
+  br label %bb5
+
+; CHECK: bb5:
+bb5:
+  br i1 %c, label %bb6, label %bb7
+
+; CHECK: bb6:
+bb6:
+  ret i32 0
+
+; CHECK: bb7:
+bb7:
+  ret i32 1
+}
Index: llvm/lib/Transforms/Scalar/JumpThreading.cpp
===================================================================
--- llvm/lib/Transforms/Scalar/JumpThreading.cpp
+++ llvm/lib/Transforms/Scalar/JumpThreading.cpp
@@ -1606,7 +1606,6 @@
   Constant *OnlyVal = nullptr;
   Constant *MultipleVal = (Constant *)(intptr_t)~0ULL;
 
-  unsigned PredWithKnownDest = 0;
   for (const auto &PredValue : PredValues) {
     BasicBlock *Pred = PredValue.second;
     if (!SeenPreds.insert(Pred).second)
@@ -1643,9 +1642,6 @@
         OnlyVal = MultipleVal;
     }
 
-    // We know where this predecessor is going.
-    ++PredWithKnownDest;
-
     // If the predecessor ends with an indirect goto, we can't change its
     // destination. Same for CallBr.
     if (isa<IndirectBrInst>(Pred->getTerminator()) ||
@@ -1663,7 +1659,7 @@
   // not thread. By doing so, we do not need to duplicate the current block and
   // also miss potential opportunities in case we dont/cant duplicate.
   if (OnlyDest && OnlyDest != MultipleDestSentinel) {
-    if (PredWithKnownDest == (size_t)pred_size(BB)) {
+    if (BB->hasNPredecessors(PredToDestList.size())) {
       bool SeenFirstBranchToOnlyDest = false;
       std::vector <DominatorTree::UpdateType> Updates;
       Updates.reserve(BB->getTerminator()->getNumSuccessors() - 1);


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D60284.193781.patch
Type: text/x-patch
Size: 2381 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20190404/904fc6cb/attachment.bin>