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

[Brian Rzycki via Phabricator via llvm-commits]

brzycki created this revision.
brzycki added reviewers: sebpop, craig.topper, hfinkel.
Herald added subscribers: jfb, hiraditya.
Herald added a project: LLVM.

The `continue` statement in `JumpThreadingPass::ProcessThreadableEdges()` correctly identifies the case when a predecessor is an `indirectbr` or `callbr` and must not be threaded. Unfortunately the bookkeeping for the remaining edges was incorrect and there are cases where a conditional branch in `BB` may be incorrectly folded based on the flawed analysis above.  This patch correctly prevents folding in these cases while not fully pessimizing other threading opportunities to the same `BB`.

Thanks to Matthias Liedtke for creating a simplified test-case and concrete failure example.

See PR40992 <https://bugs.llvm.org/show_bug.cgi?id=40992> for debug analysis.


Repository:
  rG LLVM Github Monorepo

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: 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
@@ -1643,15 +1643,15 @@
         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()) ||
         isa<CallBrInst>(Pred->getTerminator()))
       continue;
 
+    // We know where this predecessor is going.
+    ++PredWithKnownDest;
+
     PredToDestList.push_back(std::make_pair(Pred, DestBB));
   }
 


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