[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
Thu Jul 4 00:14:17 PDT 2024
https://github.com/kaadam updated https://github.com/llvm/llvm-project/pull/83394
>From 6de322c3dadd782271a109369d6ef598cb5fada2 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/6] [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 06f79dee3378f..751dc820adb0e 100644
--- a/bolt/lib/Target/AArch64/AArch64MCPlusBuilder.cpp
+++ b/bolt/lib/Target/AArch64/AArch64MCPlusBuilder.cpp
@@ -702,8 +702,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;
@@ -748,6 +760,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 e134f0d6cdfea9d7e62400b9dd20066687d50aaf 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/6] 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 b5b0a3c750d8dc7c895970e4492453499eae38a6 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/6] 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
>From 258e6bc4fe604ef16e9120d054a05e1222192af6 Mon Sep 17 00:00:00 2001
From: Adam Kallai <kadam at inf.u-szeged.hu>
Date: Tue, 18 Jun 2024 17:40:02 +0200
Subject: [PATCH 4/6] Fix the comment
---
bolt/lib/Target/AArch64/AArch64MCPlusBuilder.cpp | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/bolt/lib/Target/AArch64/AArch64MCPlusBuilder.cpp b/bolt/lib/Target/AArch64/AArch64MCPlusBuilder.cpp
index 751dc820adb0e..1a2327df1d323 100644
--- a/bolt/lib/Target/AArch64/AArch64MCPlusBuilder.cpp
+++ b/bolt/lib/Target/AArch64/AArch64MCPlusBuilder.cpp
@@ -703,7 +703,7 @@ class AArch64MCPlusBuilder : public MCPlusBuilder {
AArch64_AM::ShiftExtendType ExtendType =
AArch64_AM::getArithExtendType(OperandExtension);
if (ShiftVal != 2) {
- // TODO: handle that case where ShiftVal != 2.
+ // TODO: Handle the patten 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.
@@ -761,7 +761,7 @@ class AArch64MCPlusBuilder : public MCPlusBuilder {
}
if (DefJTBaseAdd->getOpcode() == AArch64::ADR) {
- // TODO: handle that case when we do not have adrp/add pair.
+ // TODO: Handle the pattern where there is no 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]
>From c7a9a1710bd9496ec934b4f7abad4f7b6a726909 Mon Sep 17 00:00:00 2001
From: Adam Kallai <kadam at inf.u-szeged.hu>
Date: Wed, 19 Jun 2024 16:50:52 +0200
Subject: [PATCH 5/6] Added a detailed description into the test
---
bolt/test/AArch64/test-indirect-branch.s | 13 ++++++++-----
1 file changed, 8 insertions(+), 5 deletions(-)
diff --git a/bolt/test/AArch64/test-indirect-branch.s b/bolt/test/AArch64/test-indirect-branch.s
index 2b38037ba4c2b..cb9e325654774 100644
--- a/bolt/test/AArch64/test-indirect-branch.s
+++ b/bolt/test/AArch64/test-indirect-branch.s
@@ -1,16 +1,19 @@
// 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.
+// This test checks the pattern where there is no shift amount after add
+// instruction. The pattern come from libc, it can be reproduced with
+// a 'static' built binary.
//
// 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:
+//
+// It also tests another case where there is no adrp/add pair.
+// The pattern also come from libc, and it only represents in the binary
+// if the lld linker is used to create the static binary.
+// It doesn't occur with GCC ld linker.
//
// nop => nop/adr instead of adrp/add
// adr x13, 0x215a18 <_nl_value_type_LC_COLLATE+0x50>
>From 67be07e36ae430b49267885a6efa8d0080edaf34 Mon Sep 17 00:00:00 2001
From: Adam Kallai <kadam at inf.u-szeged.hu>
Date: Wed, 3 Jul 2024 17:44:57 +0200
Subject: [PATCH 6/6] Minimize the test case for better comprehension
---
bolt/test/AArch64/test-indirect-branch.s | 67 ++++++------------------
1 file changed, 16 insertions(+), 51 deletions(-)
diff --git a/bolt/test/AArch64/test-indirect-branch.s b/bolt/test/AArch64/test-indirect-branch.s
index cb9e325654774..47a5749e9bad9 100644
--- a/bolt/test/AArch64/test-indirect-branch.s
+++ b/bolt/test/AArch64/test-indirect-branch.s
@@ -1,8 +1,15 @@
// Test how BOLT handles indirect branch sequence of instructions in
// AArch64MCPlus builder.
-// This test checks the pattern where there is no shift amount after add
-// instruction. The pattern come from libc, it can be reproduced with
-// a 'static' built binary.
+
+// 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
+
+// Pattern 1: there is no shift amount after the 'add' instruction.
//
// adr x6, 0x219fb0 <sigall_set+0x88>
// add x6, x6, x14, lsl #2
@@ -10,10 +17,8 @@
// add x6, x6, w7, sxtw => no shift amount
// br x6
//
-// It also tests another case where there is no adrp/add pair.
-// The pattern also come from libc, and it only represents in the binary
-// if the lld linker is used to create the static binary.
-// It doesn't occur with GCC ld linker.
+
+// Pattern 2: nop/adr pair is used in place of adrp/add
//
// nop => nop/adr instead of adrp/add
// adr x13, 0x215a18 <_nl_value_type_LC_COLLATE+0x50>
@@ -22,89 +27,49 @@
// 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 w1, #1
+ bl bar
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
+// CHECK: BOLT-WARNING: Failed to match indirect branch: nop/adr instead of adrp/add
.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
-
+// CHECK: BOLT-WARNING: Failed to match indirect branch: ShiftVAL != 2
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