[llvm] [BOLT][AArch64] Fixes assertion errors occurred when perf2bolt was executed (PR #83394)
Ádám Kallai via llvm-commits
llvm-commits at lists.llvm.org
Tue Jun 18 08:35:19 PDT 2024
https://github.com/kaadam updated https://github.com/llvm/llvm-project/pull/83394
>From 3c4f4f3810ae1e21ff707d78d923a61aa0cea89c Mon Sep 17 00:00:00 2001
From: Adam Kallai <kadam at inf.u-szeged.hu>
Date: Thu, 29 Feb 2024 09:45:39 +0100
Subject: [PATCH 1/3] [BOLT][AArch64] Fixes assertion errors occurred when
perf2bolt was executed
BOLT only checks for the most common indirect branch pattern during
the branch analyzation. Needs to extend the current logic with other cases
which slightly differs from the expected one.
This instruction sequences comes from libc,
it occurs when the binary is static.
As a workaround mark it as UNKNOWN branch and add support for them
in a follow up PR later.
Fixes: #83114
---
.../Target/AArch64/AArch64MCPlusBuilder.cpp | 29 ++++++++++++++--
bolt/test/AArch64/test-indirect-branch.c | 33 +++++++++++++++++++
2 files changed, 60 insertions(+), 2 deletions(-)
create mode 100644 bolt/test/AArch64/test-indirect-branch.c
diff --git a/bolt/lib/Target/AArch64/AArch64MCPlusBuilder.cpp b/bolt/lib/Target/AArch64/AArch64MCPlusBuilder.cpp
index a74eda8e4a566..0758ad491a518 100644
--- a/bolt/lib/Target/AArch64/AArch64MCPlusBuilder.cpp
+++ b/bolt/lib/Target/AArch64/AArch64MCPlusBuilder.cpp
@@ -706,8 +706,20 @@ class AArch64MCPlusBuilder : public MCPlusBuilder {
unsigned ShiftVal = AArch64_AM::getArithShiftValue(OperandExtension);
AArch64_AM::ShiftExtendType ExtendType =
AArch64_AM::getArithExtendType(OperandExtension);
- if (ShiftVal != 2)
- llvm_unreachable("Failed to match indirect branch! (fragment 2)");
+ if (ShiftVal != 2) {
+ // TODO: handle that case where ShiftVal != 2.
+ // The following code sequence below has no shift amount,
+ // the range could be 0 to 4.
+ // The pattern comes from libc, it occurs when the binary is static.
+ // adr x6, 0x219fb0 <sigall_set+0x88>
+ // add x6, x6, x14, lsl #2
+ // ldr w7, [x6]
+ // add x6, x6, w7, sxtw => no shift amount
+ // br x6
+ errs() << "BOLT-WARNING: "
+ "Failed to match indirect branch: ShiftVAL != 2 \n";
+ return false;
+ }
if (ExtendType == AArch64_AM::SXTB)
ScaleValue = 1LL;
@@ -752,6 +764,19 @@ class AArch64MCPlusBuilder : public MCPlusBuilder {
return true;
}
+ if (DefJTBaseAdd->getOpcode() == AArch64::ADR) {
+ // TODO: handle that case when we do not have adrp/add pair.
+ // It also occurs when the binary is static.
+ // adr x13, 0x215a18 <_nl_value_type_LC_COLLATE+0x50>
+ // ldrh w13, [x13, w12, uxtw #1]
+ // adr x12, 0x247b30 <__gettextparse+0x5b0>
+ // add x13, x12, w13, sxth #2
+ // br x13
+ errs() << "BOLT-WARNING: Failed to match indirect branch: "
+ "nop/adr instead of adrp/add \n";
+ return false;
+ }
+
assert(DefJTBaseAdd->getOpcode() == AArch64::ADDXri &&
"Failed to match jump table base address pattern! (1)");
diff --git a/bolt/test/AArch64/test-indirect-branch.c b/bolt/test/AArch64/test-indirect-branch.c
new file mode 100644
index 0000000000000..97bd8265feb34
--- /dev/null
+++ b/bolt/test/AArch64/test-indirect-branch.c
@@ -0,0 +1,33 @@
+// Test how BOLT handles indirect branch sequence of instructions in
+// AArch64MCPlus builder.
+// This test checks that case when we have no shift amount after add
+// instruction. This pattern comes from libc, so needs to build '-static'
+// binary to reproduce the issue easily.
+//
+// adr x6, 0x219fb0 <sigall_set+0x88>
+// add x6, x6, x14, lsl #2
+// ldr w7, [x6]
+// add x6, x6, w7, sxtw => no shift amount
+// br x6
+// It also tests another case when we use '-fuse-ld=lld' along with '-static'
+// which produces the following sequence of intsructions:
+//
+// nop => nop/adr instead of adrp/add
+// adr x13, 0x215a18 <_nl_value_type_LC_COLLATE+0x50>
+// ldrh w13, [x13, w12, uxtw #1]
+// adr x12, 0x247b30 <__gettextparse+0x5b0>
+// add x13, x12, w13, sxth #2
+// br x13
+
+// clang-format off
+
+// REQUIRES: system-linux
+// RUN: %clang %s -o %t.exe -Wl,-q -static -fuse-ld=lld \
+// RUN: --target=aarch64-unknown-linux-gnu
+// RUN: llvm-bolt %t.exe -o %t.bolt --print-cfg \
+// RUN: -v=1 2>&1 | FileCheck --match-full-lines %s
+
+// CHECK: BOLT-WARNING: Failed to match indirect branch: nop/adr instead of adrp/add
+// CHECK: BOLT-WARNING: Failed to match indirect branch: ShiftVAL != 2
+
+int main() { return 42; }
>From c24be4640227cbaf59a8a8ed4ef929bb8750e20b Mon Sep 17 00:00:00 2001
From: Adam Kallai <kadam at inf.u-szeged.hu>
Date: Wed, 12 Jun 2024 15:17:00 +0200
Subject: [PATCH 2/3] Enable this test only on AArch64 target
---
bolt/test/AArch64/test-indirect-branch.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/bolt/test/AArch64/test-indirect-branch.c b/bolt/test/AArch64/test-indirect-branch.c
index 97bd8265feb34..8c8a15af72dbb 100644
--- a/bolt/test/AArch64/test-indirect-branch.c
+++ b/bolt/test/AArch64/test-indirect-branch.c
@@ -21,7 +21,7 @@
// clang-format off
-// REQUIRES: system-linux
+// REQUIRES: system-linux,target=aarch64{{.*}}
// RUN: %clang %s -o %t.exe -Wl,-q -static -fuse-ld=lld \
// RUN: --target=aarch64-unknown-linux-gnu
// RUN: llvm-bolt %t.exe -o %t.bolt --print-cfg \
>From 920aad45e4813f4c528b7ac4d64b08c562826f29 Mon Sep 17 00:00:00 2001
From: Adam Kallai <kadam at inf.u-szeged.hu>
Date: Tue, 18 Jun 2024 17:28:20 +0200
Subject: [PATCH 3/3] Added assembly test
---
bolt/test/AArch64/test-indirect-branch.c | 33 -------
bolt/test/AArch64/test-indirect-branch.s | 107 +++++++++++++++++++++++
2 files changed, 107 insertions(+), 33 deletions(-)
delete mode 100644 bolt/test/AArch64/test-indirect-branch.c
create mode 100644 bolt/test/AArch64/test-indirect-branch.s
diff --git a/bolt/test/AArch64/test-indirect-branch.c b/bolt/test/AArch64/test-indirect-branch.c
deleted file mode 100644
index 8c8a15af72dbb..0000000000000
--- a/bolt/test/AArch64/test-indirect-branch.c
+++ /dev/null
@@ -1,33 +0,0 @@
-// Test how BOLT handles indirect branch sequence of instructions in
-// AArch64MCPlus builder.
-// This test checks that case when we have no shift amount after add
-// instruction. This pattern comes from libc, so needs to build '-static'
-// binary to reproduce the issue easily.
-//
-// adr x6, 0x219fb0 <sigall_set+0x88>
-// add x6, x6, x14, lsl #2
-// ldr w7, [x6]
-// add x6, x6, w7, sxtw => no shift amount
-// br x6
-// It also tests another case when we use '-fuse-ld=lld' along with '-static'
-// which produces the following sequence of intsructions:
-//
-// nop => nop/adr instead of adrp/add
-// adr x13, 0x215a18 <_nl_value_type_LC_COLLATE+0x50>
-// ldrh w13, [x13, w12, uxtw #1]
-// adr x12, 0x247b30 <__gettextparse+0x5b0>
-// add x13, x12, w13, sxth #2
-// br x13
-
-// clang-format off
-
-// REQUIRES: system-linux,target=aarch64{{.*}}
-// RUN: %clang %s -o %t.exe -Wl,-q -static -fuse-ld=lld \
-// RUN: --target=aarch64-unknown-linux-gnu
-// RUN: llvm-bolt %t.exe -o %t.bolt --print-cfg \
-// RUN: -v=1 2>&1 | FileCheck --match-full-lines %s
-
-// CHECK: BOLT-WARNING: Failed to match indirect branch: nop/adr instead of adrp/add
-// CHECK: BOLT-WARNING: Failed to match indirect branch: ShiftVAL != 2
-
-int main() { return 42; }
diff --git a/bolt/test/AArch64/test-indirect-branch.s b/bolt/test/AArch64/test-indirect-branch.s
new file mode 100644
index 0000000000000..2b38037ba4c2b
--- /dev/null
+++ b/bolt/test/AArch64/test-indirect-branch.s
@@ -0,0 +1,107 @@
+// Test how BOLT handles indirect branch sequence of instructions in
+// AArch64MCPlus builder.
+// This test checks that case when we have no shift amount after add
+// instruction. This pattern comes from libc, so needs to build '-static'
+// binary to reproduce the issue easily.
+//
+// adr x6, 0x219fb0 <sigall_set+0x88>
+// add x6, x6, x14, lsl #2
+// ldr w7, [x6]
+// add x6, x6, w7, sxtw => no shift amount
+// br x6
+// It also tests another case when we use '-fuse-ld=lld' along with '-static'
+// which produces the following sequence of intsructions:
+//
+// nop => nop/adr instead of adrp/add
+// adr x13, 0x215a18 <_nl_value_type_LC_COLLATE+0x50>
+// ldrh w13, [x13, w12, uxtw #1]
+// adr x12, 0x247b30 <__gettextparse+0x5b0>
+// add x13, x12, w13, sxth #2
+// br x13
+
+// clang-format off
+
+// REQUIRES: system-linux
+// RUN: llvm-mc -filetype=obj -triple aarch64-unknown-unknown %s -o %t.o
+// RUN: %clang %cflags --target=aarch64-unknown-linux %t.o -o %t.exe -Wl,-q
+// RUN: llvm-bolt %t.exe -o %t.bolt --print-cfg \
+// RUN: -v=1 2>&1 | FileCheck %s
+
+// CHECK: BOLT-WARNING: Failed to match indirect branch: nop/adr instead of adrp/add
+// CHECK: BOLT-WARNING: Failed to match indirect branch: ShiftVAL != 2
+
+
+ .section .text
+ .align 4
+ .globl _start
+ .type _start, %function
+_start:
+ bl bar
+ bl end
+ mov x0, #4
+ mov w8, #93
+ svc #0
+
+bar:
+ mov w1, #3
+ cmp x1, #0
+ b.eq end
+ nop
+ adr x3, jump_table
+ ldrh w3, [x3, x1, lsl #1]
+ adr x1, .case0
+ add x3, x1, w3, sxth #2
+ br x3
+.case0:
+ mov w0, #1
+ ret
+.case1:
+ mov w0, #2
+ ret
+.case3:
+ mov w0, #3
+ ret
+.case4:
+ nop
+ mov x1, #0
+ adr x3, datatable
+ add x3, x3, x1, lsl #2
+ ldr w2, [x3]
+ add x3, x3, w2, sxtw
+ br x3
+ nop
+ mov w0, #4
+ ret
+.case7:
+ mov w0, #4
+ ret
+
+foo1:
+ ret
+
+foo2:
+ add w0, w0, #3
+ ret
+
+foo3:
+ add w0, w0, #3
+ ret
+
+end:
+ add x0, x0, #99
+ ret
+
+ .section .rodata,"a", at progbits
+jump_table:
+ .hword (.case0-.case0)>>2
+ .hword (.case1-.case0)>>2
+ .hword (.case3-.case0)>>2
+ .hword (.case4-.case0)>>2
+ .hword (.case7-.case0)>>2
+
+
+datatable:
+ .word foo1-datatable
+ .word foo2-datatable
+ .word foo3-datatable
+ .word 20
More information about the llvm-commits
mailing list