Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bail out jump threading on indirect branches only #117778

Merged
merged 1 commit into from
Nov 26, 2024

Conversation

hiraditya
Copy link
Collaborator

Remove check for PHI in pred as pointed out in #103688
Reduced the testcase to remove redundant phi in pred

Fixes: #102351

Remove check for PHI in pred as pointed out in llvm#103688
Reduced the testcase to remove redundant phi in pred
Fixes: 102351
@llvmbot
Copy link
Member

llvmbot commented Nov 26, 2024

@llvm/pr-subscribers-llvm-transforms

Author: AdityaK (hiraditya)

Changes

Remove check for PHI in pred as pointed out in #103688
Reduced the testcase to remove redundant phi in pred

Fixes: #102351


Full diff: https://github.com/llvm/llvm-project/pull/117778.diff

2 Files Affected:

  • (modified) llvm/lib/Transforms/Utils/Local.cpp (+1-2)
  • (modified) llvm/test/Transforms/SimplifyCFG/switch-branch-fold-indirectbr-102351.ll (+59-82)
diff --git a/llvm/lib/Transforms/Utils/Local.cpp b/llvm/lib/Transforms/Utils/Local.cpp
index d574095d75c58c..ec01e940a94c57 100644
--- a/llvm/lib/Transforms/Utils/Local.cpp
+++ b/llvm/lib/Transforms/Utils/Local.cpp
@@ -1034,8 +1034,7 @@ CanRedirectPredsOfEmptyBBToSucc(BasicBlock *BB, BasicBlock *Succ,
     return false;
 
   if (any_of(BBPreds, [](const BasicBlock *Pred) {
-        return isa<PHINode>(Pred->begin()) &&
-               isa<IndirectBrInst>(Pred->getTerminator());
+        return isa<IndirectBrInst>(Pred->getTerminator());
       }))
     return false;
 
diff --git a/llvm/test/Transforms/SimplifyCFG/switch-branch-fold-indirectbr-102351.ll b/llvm/test/Transforms/SimplifyCFG/switch-branch-fold-indirectbr-102351.ll
index 03aee68fa4248c..d3713be8358db4 100644
--- a/llvm/test/Transforms/SimplifyCFG/switch-branch-fold-indirectbr-102351.ll
+++ b/llvm/test/Transforms/SimplifyCFG/switch-branch-fold-indirectbr-102351.ll
@@ -1,104 +1,81 @@
-; NOTE: Assertions have been autogenerated by utils/update_test_checks.py UTC_ARGS: --version 5
+; NOTE: Assertions have been autogenerated by utils/update_test_checks.py UTC_ARGS: --prefix-filecheck-ir-name pref --version 5
 ; RUN: opt < %s -passes=simplifycfg -S | FileCheck %s
 
-target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-i128:128-f80:128-n8:16:32:64-S128"
-target triple = "x86_64-unknown-linux-gnu"
-
-define dso_local noundef i32 @main() {
-; CHECK-LABEL: define dso_local noundef i32 @main() {
+define i32 @foo.1(i32 %arg, ptr %arg1) {
+; CHECK-LABEL: define i32 @foo.1(
+; CHECK-SAME: i32 [[ARG:%.*]], ptr [[ARG1:%.*]]) {
 ; CHECK-NEXT:  [[BB:.*]]:
 ; CHECK-NEXT:    [[ALLOCA:%.*]] = alloca [2 x ptr], align 16
-; CHECK-NEXT:    store ptr blockaddress(@main, %[[BB4:.*]]), ptr [[ALLOCA]], align 16, !tbaa [[TBAA0:![0-9]+]]
+; CHECK-NEXT:    store ptr blockaddress(@foo.1, %[[BB8:.*]]), ptr [[ALLOCA]], align 16
 ; CHECK-NEXT:    [[GETELEMENTPTR:%.*]] = getelementptr inbounds [2 x ptr], ptr [[ALLOCA]], i64 0, i64 1
-; CHECK-NEXT:    store ptr blockaddress(@main, %[[BB10:.*]]), ptr [[GETELEMENTPTR]], align 8, !tbaa [[TBAA0]]
-; CHECK-NEXT:    br label %[[BB1:.*]]
-; CHECK:       [[BB1]]:
-; CHECK-NEXT:    [[PHI:%.*]] = phi i32 [ 0, %[[BB]] ], [ [[PHI8:%.*]], %[[BB7:.*]] ]
-; CHECK-NEXT:    [[PHI2:%.*]] = phi i32 [ 0, %[[BB]] ], [ [[PHI9:%.*]], %[[BB7]] ]
-; CHECK-NEXT:    switch i32 [[PHI]], label %[[BB7]] [
-; CHECK-NEXT:      i32 0, label %[[BB12:.*]]
-; CHECK-NEXT:      i32 1, label %[[BB4]]
-; CHECK-NEXT:      i32 2, label %[[BB6:.*]]
+; CHECK-NEXT:    store ptr blockaddress(@foo.1, %[[BB16:.*]]), ptr [[GETELEMENTPTR]], align 8
+; CHECK-NEXT:    br label %[[PREFBB2:.*]]
+; CHECK:       [[PREFBB2]]:
+; CHECK-NEXT:    [[PHI:%.*]] = phi i32 [ 0, %[[BB]] ], [ [[PHI14:%.*]], %[[BB13:.*]] ]
+; CHECK-NEXT:    [[PHI3:%.*]] = phi i32 [ 0, %[[BB]] ], [ [[PHI15:%.*]], %[[BB13]] ]
+; CHECK-NEXT:    switch i32 [[PHI]], label %[[BB13]] [
+; CHECK-NEXT:      i32 0, label %[[PREFBB18:.*]]
+; CHECK-NEXT:      i32 1, label %[[BB8]]
+; CHECK-NEXT:      i32 2, label %[[PREFBB11:.*]]
 ; CHECK-NEXT:    ]
-; CHECK:       [[BB4]]:
-; CHECK-NEXT:    [[PHI5:%.*]] = phi i32 [ [[PHI13:%.*]], %[[BB12]] ], [ [[PHI2]], %[[BB1]] ]
-; CHECK-NEXT:    br label %[[BB7]]
-; CHECK:       [[BB6]]:
-; CHECK-NEXT:    [[CALL:%.*]] = call i32 @foo(i32 noundef [[PHI2]])
-; CHECK-NEXT:    [[ADD:%.*]] = add nsw i32 [[PHI2]], 1
-; CHECK-NEXT:    br label %[[BB12]]
-; CHECK:       [[BB7]]:
-; CHECK-NEXT:    [[PHI8]] = phi i32 [ [[PHI]], %[[BB1]] ], [ 2, %[[BB4]] ]
-; CHECK-NEXT:    [[PHI9]] = phi i32 [ [[PHI2]], %[[BB1]] ], [ [[PHI5]], %[[BB4]] ]
-; CHECK-NEXT:    br label %[[BB1]], !llvm.loop [[LOOP4:![0-9]+]]
-; CHECK:       [[BB10]]:
-; CHECK-NEXT:    [[CALL11:%.*]] = call i32 @foo(i32 noundef [[PHI13]])
+; CHECK:       [[BB8]]:
+; CHECK-NEXT:    [[PHI10:%.*]] = phi i32 [ [[ARG]], %[[PREFBB18]] ], [ [[PHI3]], %[[PREFBB2]] ]
+; CHECK-NEXT:    br label %[[BB13]]
+; CHECK:       [[PREFBB11]]:
+; CHECK-NEXT:    [[CALL:%.*]] = call i32 @wombat(i32 noundef [[PHI3]])
+; CHECK-NEXT:    [[ADD:%.*]] = add nsw i32 [[PHI3]], 1
+; CHECK-NEXT:    br label %[[PREFBB18]]
+; CHECK:       [[BB13]]:
+; CHECK-NEXT:    [[PHI14]] = phi i32 [ [[PHI]], %[[PREFBB2]] ], [ 2, %[[BB8]] ]
+; CHECK-NEXT:    [[PHI15]] = phi i32 [ [[PHI3]], %[[PREFBB2]] ], [ [[PHI10]], %[[BB8]] ]
+; CHECK-NEXT:    br label %[[PREFBB2]]
+; CHECK:       [[BB16]]:
+; CHECK-NEXT:    [[CALL17:%.*]] = call i32 @wombat(i32 noundef [[ARG]])
 ; CHECK-NEXT:    ret i32 0
-; CHECK:       [[BB12]]:
-; CHECK-NEXT:    [[PHI13]] = phi i32 [ [[ADD]], %[[BB6]] ], [ [[PHI2]], %[[BB1]] ]
-; CHECK-NEXT:    [[SEXT:%.*]] = sext i32 [[PHI13]] to i64
-; CHECK-NEXT:    [[GETELEMENTPTR14:%.*]] = getelementptr inbounds [2 x ptr], ptr [[ALLOCA]], i64 0, i64 [[SEXT]]
-; CHECK-NEXT:    [[LOAD:%.*]] = load ptr, ptr [[GETELEMENTPTR14]], align 8, !tbaa [[TBAA0]]
-; CHECK-NEXT:    indirectbr ptr [[LOAD]], [label %[[BB4]], label %bb10]
+; CHECK:       [[PREFBB18]]:
+; CHECK-NEXT:    [[LOAD:%.*]] = load ptr, ptr [[ARG1]], align 8
+; CHECK-NEXT:    indirectbr ptr [[LOAD]], [label %[[BB8]], label %bb16]
 ;
 bb:
   %alloca = alloca [2 x ptr], align 16
-  store ptr blockaddress(@main, %bb4), ptr %alloca, align 16, !tbaa !0
+  store ptr blockaddress(@foo.1, %bb8), ptr %alloca, align 16
   %getelementptr = getelementptr inbounds [2 x ptr], ptr %alloca, i64 0, i64 1
-  store ptr blockaddress(@main, %bb10), ptr %getelementptr, align 8, !tbaa !0
-  br label %bb1
+  store ptr blockaddress(@foo.1, %bb16), ptr %getelementptr, align 8
+  br label %bb2
 
-bb1:                                              ; preds = %bb7, %bb
-  %phi = phi i32 [ 0, %bb ], [ %phi8, %bb7 ]
-  %phi2 = phi i32 [ 0, %bb ], [ %phi9, %bb7 ]
-  switch i32 %phi, label %bb7 [
-  i32 0, label %bb3
-  i32 1, label %bb4
-  i32 2, label %bb6
+bb2:                                              ; preds = %bb13, %bb
+  %phi = phi i32 [ 0, %bb ], [ %phi14, %bb13 ]
+  %phi3 = phi i32 [ 0, %bb ], [ %phi15, %bb13 ]
+  switch i32 %phi, label %bb13 [
+  i32 0, label %bb5
+  i32 1, label %bb8
+  i32 2, label %bb11
   ]
 
-bb3:                                              ; preds = %bb1
-  br label %bb12
+bb5:                                              ; preds = %bb2
+  br label %bb18
 
-bb4:                                              ; preds = %bb12, %bb1
-  %phi5 = phi i32 [ %phi13, %bb12 ], [ %phi2, %bb1 ]
-  br label %bb7
+bb8:                                              ; preds = %bb18, %bb2
+  %phi10 = phi i32 [ %arg, %bb18 ], [ %phi3, %bb2 ]
+  br label %bb13
 
-bb6:                                              ; preds = %bb1
-  %call = call i32 @foo(i32 noundef %phi2)
-  %add = add nsw i32 %phi2, 1
-  br label %bb12
+bb11:                                             ; preds = %bb2
+  %call = call i32 @wombat(i32 noundef %phi3)
+  %add = add nsw i32 %phi3, 1
+  br label %bb18
 
-bb7:                                              ; preds = %bb4, %bb1
-  %phi8 = phi i32 [ %phi, %bb1 ], [ 2, %bb4 ]
-  %phi9 = phi i32 [ %phi2, %bb1 ], [ %phi5, %bb4 ]
-  br label %bb1, !llvm.loop !4
+bb13:                                             ; preds = %bb8, %bb2
+  %phi14 = phi i32 [ %phi, %bb2 ], [ 2, %bb8 ]
+  %phi15 = phi i32 [ %phi3, %bb2 ], [ %phi10, %bb8 ]
+  br label %bb2
 
-bb10:                                             ; preds = %bb12
-  %call11 = call i32 @foo(i32 noundef %phi13)
+bb16:                                             ; preds = %bb18
+  %call17 = call i32 @wombat(i32 noundef %arg)
   ret i32 0
 
-bb12:                                             ; preds = %bb6, %bb3
-  %phi13 = phi i32 [ %add, %bb6 ], [ %phi2, %bb3 ]
-  %sext = sext i32 %phi13 to i64
-  %getelementptr14 = getelementptr inbounds [2 x ptr], ptr %alloca, i64 0, i64 %sext
-  %load = load ptr, ptr %getelementptr14, align 8, !tbaa !0
-  indirectbr ptr %load, [label %bb4, label %bb10]
+bb18:                                             ; preds = %bb11, %bb5
+  %load = load ptr, ptr %arg1, align 8
+  indirectbr ptr %load, [label %bb8, label %bb16]
 }
 
-declare i32 @foo(i32)
-
-!0 = !{!1, !1, i64 0}
-!1 = !{!"any pointer", !2, i64 0}
-!2 = !{!"omnipotent char", !3, i64 0}
-!3 = !{!"Simple C++ TBAA"}
-!4 = !{!5, !5, i64 0}
-!5 = !{!"int", !2, i64 0}
-;.
-; CHECK: [[TBAA0]] = !{[[META1:![0-9]+]], [[META1]], i64 0}
-; CHECK: [[META1]] = !{!"any pointer", [[META2:![0-9]+]], i64 0}
-; CHECK: [[META2]] = !{!"omnipotent char", [[META3:![0-9]+]], i64 0}
-; CHECK: [[META3]] = !{!"Simple C++ TBAA"}
-; CHECK: [[LOOP4]] = !{[[META5:![0-9]+]], [[META5]], i64 0}
-; CHECK: [[META5]] = !{!"int", [[META2]], i64 0}
-;.
+declare i32 @wombat(i32)

Copy link
Contributor

@nikic nikic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks!

@hiraditya hiraditya merged commit 39601a6 into llvm:main Nov 26, 2024
10 checks passed
@hiraditya hiraditya deleted the simplifycfg branch November 26, 2024 22:57
@llvm-ci
Copy link
Collaborator

llvm-ci commented Nov 26, 2024

LLVM Buildbot has detected a new failure on builder ml-opt-rel-x86-64 running on ml-opt-rel-x86-64-b2 while building llvm at step 6 "test-build-unified-tree-check-all".

Full details are available at: https://lab.llvm.org/buildbot/#/builders/185/builds/9222

Here is the relevant piece of the build log for the reference
Step 6 (test-build-unified-tree-check-all) failure: test (failure)
******************** TEST 'LLVM :: ExecutionEngine/OrcLazy/multiple-compile-threads-basic.ll' FAILED ********************
Exit Code: 2

Command Output (stderr):
--
RUN: at line 1: /b/ml-opt-rel-x86-64-b1/build/bin/lli -jit-kind=orc-lazy -compile-threads=2 -thread-entry hello /b/ml-opt-rel-x86-64-b1/llvm-project/llvm/test/ExecutionEngine/OrcLazy/multiple-compile-threads-basic.ll | /b/ml-opt-rel-x86-64-b1/build/bin/FileCheck /b/ml-opt-rel-x86-64-b1/llvm-project/llvm/test/ExecutionEngine/OrcLazy/multiple-compile-threads-basic.ll
+ /b/ml-opt-rel-x86-64-b1/build/bin/lli -jit-kind=orc-lazy -compile-threads=2 -thread-entry hello /b/ml-opt-rel-x86-64-b1/llvm-project/llvm/test/ExecutionEngine/OrcLazy/multiple-compile-threads-basic.ll
+ /b/ml-opt-rel-x86-64-b1/build/bin/FileCheck /b/ml-opt-rel-x86-64-b1/llvm-project/llvm/test/ExecutionEngine/OrcLazy/multiple-compile-threads-basic.ll
lli: /b/ml-opt-rel-x86-64-b1/llvm-project/llvm/include/llvm/ExecutionEngine/Orc/SymbolStringPool.h:282: llvm::orc::SymbolStringPool::~SymbolStringPool(): Assertion `Pool.empty() && "Dangling references at pool destruction time"' failed.
PLEASE submit a bug report to https://github.com/llvm/llvm-project/issues/ and include the crash backtrace.
Stack dump:
0.	Program arguments: /b/ml-opt-rel-x86-64-b1/build/bin/lli -jit-kind=orc-lazy -compile-threads=2 -thread-entry hello /b/ml-opt-rel-x86-64-b1/llvm-project/llvm/test/ExecutionEngine/OrcLazy/multiple-compile-threads-basic.ll
 #0 0x00005650dbf05188 llvm::sys::PrintStackTrace(llvm::raw_ostream&, int) (/b/ml-opt-rel-x86-64-b1/build/bin/lli+0x15d7188)
 #1 0x00005650dbf0256c SignalHandler(int) Signals.cpp:0:0
 #2 0x00007f5e183f2140 __restore_rt (/lib/x86_64-linux-gnu/libpthread.so.0+0x13140)
 #3 0x00007f5e17ef3d51 raise (/lib/x86_64-linux-gnu/libc.so.6+0x38d51)
 #4 0x00007f5e17edd537 abort (/lib/x86_64-linux-gnu/libc.so.6+0x22537)
 #5 0x00007f5e17edd40f (/lib/x86_64-linux-gnu/libc.so.6+0x2240f)
 #6 0x00007f5e17eec6d2 (/lib/x86_64-linux-gnu/libc.so.6+0x316d2)
 #7 0x00005650dae3e9df (/b/ml-opt-rel-x86-64-b1/build/bin/lli+0x5109df)
 #8 0x00005650db9c55ca llvm::orc::ExecutorProcessControl::~ExecutorProcessControl() (/b/ml-opt-rel-x86-64-b1/build/bin/lli+0x10975ca)
 #9 0x00005650db9c57f3 llvm::orc::SelfExecutorProcessControl::~SelfExecutorProcessControl() crtstuff.c:0:0
#10 0x00005650db9142e6 llvm::orc::ExecutionSession::~ExecutionSession() (/b/ml-opt-rel-x86-64-b1/build/bin/lli+0xfe62e6)
#11 0x00005650db955a0a llvm::orc::LLJIT::~LLJIT() (/b/ml-opt-rel-x86-64-b1/build/bin/lli+0x1027a0a)
#12 0x00005650db955ba1 llvm::orc::LLLazyJIT::~LLLazyJIT() crtstuff.c:0:0
#13 0x00005650dae48c2b runOrcJIT(char const*) (/b/ml-opt-rel-x86-64-b1/build/bin/lli+0x51ac2b)
#14 0x00005650dadb66e5 main (/b/ml-opt-rel-x86-64-b1/build/bin/lli+0x4886e5)
#15 0x00007f5e17eded7a __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x23d7a)
#16 0x00005650dae344aa _start (/b/ml-opt-rel-x86-64-b1/build/bin/lli+0x5064aa)
FileCheck error: '<stdin>' is empty.
FileCheck command line:  /b/ml-opt-rel-x86-64-b1/build/bin/FileCheck /b/ml-opt-rel-x86-64-b1/llvm-project/llvm/test/ExecutionEngine/OrcLazy/multiple-compile-threads-basic.ll

--

********************


@nikic nikic added this to the LLVM 19.X Release milestone Nov 27, 2024
@nikic
Copy link
Contributor

nikic commented Nov 27, 2024

/cherry-pick 3c9022c 39601a6

@llvmbot
Copy link
Member

llvmbot commented Nov 27, 2024

/pull-request #117869

tru pushed a commit to llvmbot/llvm-project that referenced this pull request Dec 2, 2024
Remove check for PHI in pred as pointed out in llvm#103688
Reduced the testcase to remove redundant phi in pred

Fixes: llvm#102351
(cherry picked from commit 39601a6)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Development

Successfully merging this pull request may close these issues.

Miscompile in code with indirect gotos
4 participants