-
Notifications
You must be signed in to change notification settings - Fork 15.3k
Revert "[BOLT][BTI] Skip inlining BasicBlocks containing indirect tailcalls" #169881
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
Revert "[BOLT][BTI] Skip inlining BasicBlocks containing indirect tailcalls" #169881
Conversation
…lcalls (…" This reverts commit 5d6d743.
|
@llvm/pr-subscribers-bolt Author: Gergely Bálint (bgergely0) ChangesReverts llvm/llvm-project#168403 The attached lit test is failing in some build configurations. Full diff: https://github.com/llvm/llvm-project/pull/169881.diff 2 Files Affected:
diff --git a/bolt/lib/Passes/Inliner.cpp b/bolt/lib/Passes/Inliner.cpp
index 0740fcef9102b..5a7d02a34b4d8 100644
--- a/bolt/lib/Passes/Inliner.cpp
+++ b/bolt/lib/Passes/Inliner.cpp
@@ -491,32 +491,6 @@ bool Inliner::inlineCallsInFunction(BinaryFunction &Function) {
}
}
- // AArch64 BTI:
- // If the callee has an indirect tailcall (BR), we would transform it to
- // an indirect call (BLR) in InlineCall. Because of this, we would have to
- // update the BTI at the target of the tailcall. However, these targets
- // are not known. Instead, we skip inlining blocks with indirect
- // tailcalls.
- auto HasIndirectTailCall = [&](const BinaryFunction &BF) -> bool {
- for (const auto &BB : BF) {
- for (const auto &II : BB) {
- if (BC.MIB->isIndirectBranch(II) && BC.MIB->isTailCall(II)) {
- return true;
- }
- }
- }
- return false;
- };
-
- if (BC.isAArch64() && BC.usesBTI() &&
- HasIndirectTailCall(*TargetFunction)) {
- ++InstIt;
- LLVM_DEBUG(dbgs() << "BOLT-DEBUG: Skipping inlining block with tailcall"
- << " in " << Function << " : " << BB->getName()
- << " to keep BTIs consistent.\n");
- continue;
- }
-
LLVM_DEBUG(dbgs() << "BOLT-DEBUG: inlining call to " << *TargetFunction
<< " in " << Function << " : " << BB->getName()
<< ". Count: " << BB->getKnownExecutionCount()
diff --git a/bolt/test/AArch64/inline-bti.s b/bolt/test/AArch64/inline-bti.s
deleted file mode 100644
index c0b9ebe632b6c..0000000000000
--- a/bolt/test/AArch64/inline-bti.s
+++ /dev/null
@@ -1,39 +0,0 @@
-## This test checks that for AArch64 binaries with BTI, we do not inline blocks with indirect tailcalls.
-
-# REQUIRES: system-linux
-
-# RUN: llvm-mc -filetype=obj -triple aarch64-unknown-unknown %s -o %t.o
-# RUN: %clang %cflags -O0 %t.o -o %t.exe -Wl,-q -Wl,-z,force-bti
-# RUN: llvm-bolt --inline-all %t.exe -o %t.bolt --debug 2>&1 | FileCheck %s
-
-# For BTI, we should not inline foo.
-# CHECK: BOLT-DEBUG: Skipping inlining block with tailcall in _Z3barP1A : .LBB01 to keep BTIs consistent.
-# CHECK-NOT: BOLT-INFO: inlined {{[0-9]+}} calls at {{[0-9]+}} call sites in {{[0-9]+}} iteration(s). Change in binary size: {{[0-9]+}} bytes.
-
- .text
- .globl _Z3fooP1A
- .type _Z3fooP1A,@function
-_Z3fooP1A:
- ldr x8, [x0]
- ldr w0, [x8]
- br x30
- .size _Z3fooP1A, .-_Z3fooP1A
-
- .globl _Z3barP1A
- .type _Z3barP1A,@function
-_Z3barP1A:
- stp x29, x30, [sp, #-16]!
- mov x29, sp
- bl _Z3fooP1A
- mul w0, w0, w0
- ldp x29, x30, [sp], #16
- ret
- .size _Z3barP1A, .-_Z3barP1A
-
- .globl main
- .p2align 2
- .type main,@function
-main:
- mov w0, wzr
- ret
- .size main, .-main
|
paschalis-mpeis
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Was this a quick revert? Can reland by requiring asserts?
|
Yeah, I think the assert requirement will be it. I just didn't want to keep the failing test in while I investigate. |
…ilcalls" (#169881) This reverts commit 9bffb10. Fix: added assertions to the requirements of the test -------- Original commit message: In the Inliner pass, tailcalls are converted to calls in the inlined BasicBlock. If the tailcall is indirect, the `BR` is converted to `BLR`. These instructions require different BTI landing pads at their targets. As the targets of indirect tailcalls are unknown, inlining such blocks is unsound for BTI: they should be skipped instead.
Reverts #168403
The attached lit test is failing in some build configurations.