[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
Fri Jul 5 03:07:26 PDT 2024
https://github.com/kaadam updated https://github.com/llvm/llvm-project/pull/83394
>From 977f649fb9371e1f7efe141f051d8de3d8304c6f 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 e1e989402f648fd9ad9cbff3168fbbe82bc8b616 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 f486e7fafa830b0040745340fdecfd20b741f960 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 5bb02a1a22f3baa722cfd88c843a963452f2ea0c 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 e72ceab0a0c4e97e1efab4b25a14f897448cf20a 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 499c3fc16862464d1a00b852f20ab448e39098a1 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 | 119 +++++++++--------------
1 file changed, 46 insertions(+), 73 deletions(-)
diff --git a/bolt/test/AArch64/test-indirect-branch.s b/bolt/test/AArch64/test-indirect-branch.s
index cb9e325654774..168e50c8f47f5 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 --strict\
+// 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,57 @@
// 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
+ bl test1
+ bl test2
+// 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
+// Pattern 1
+// CHECK: BOLT-WARNING: Failed to match indirect branch: ShiftVAL != 2
+ .globl test1
+ .type test1, %function
+test1:
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:
+test1_0:
ret
-
-foo2:
- add w0, w0, #3
+test1_1:
ret
-
-foo3:
- add w0, w0, #3
+test1_2:
ret
-end:
- add x0, x0, #99
+// Pattern 2
+// CHECK: BOLT-WARNING: Failed to match indirect branch: nop/adr instead of adrp/add
+ .globl test2
+ .type test2, %function
+test2:
+ nop
+ adr x3, jump_table
+ ldrh w3, [x3, x1, lsl #1]
+ adr x1, test2_0
+ add x3, x1, w3, sxth #2
+ br x3
+test2_0:
+ ret
+test2_1:
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
+ .word test1_0-datatable
+ .word test1_1-datatable
+ .word test1_2-datatable
+
+jump_table:
+ .hword (test2_0-test2_0)>>2
+ .hword (test2_1-test2_0)>>2
More information about the llvm-commits
mailing list