[LLD][ARM] Arm v6-m should not use short Thunks. #118111
Conversation
Thumb short thunks use the B.w instruction. This instruction is not present on Arm v6-m so we should prevent these targets from using short-thunks. We want to permit Arm v8-m.base targets to continue using short thunks as it does have the B.w instruction despite not implementing all of Thumb 2. Add a check to see if the Movt and Movw instructions are present before enabling short thunks for Thumb. The v6-m architecture has J1J2BranchEncoding, but it does not have Movt and Movw, whereas v8-m.base has both. The memory map and limited flash size of an Arm v6-m CPU makes a short thunk very unlikely in practice, but it is worth getting it right just in case.
|
@llvm/pr-subscribers-lld-elf @llvm/pr-subscribers-lld Author: Peter Smith (smithp35) ChangesThumb short thunks use the B.w instruction. This instruction is not present on Arm v6-m so we should prevent these targets from using short-thunks. We want to permit Arm v8-m.base targets to continue using short thunks as it does have the B.w instruction despite not implementing all of Thumb 2. Add a check to see if the Movt and Movw instructions are present before enabling short thunks for Thumb. The v6-m architecture has J1J2BranchEncoding, but it does not have Movt and Movw, whereas v8-m.base has both. The memory map and limited flash size of an Arm v6-m CPU makes a short thunk very unlikely in practice, but it is worth getting it right just in case. Full diff: https://github.com/llvm/llvm-project/pull/118111.diff 2 Files Affected:
diff --git a/lld/ELF/Thunks.cpp b/lld/ELF/Thunks.cpp
index 629ce356ce2e7d..4e4e0684a3f596 100644
--- a/lld/ELF/Thunks.cpp
+++ b/lld/ELF/Thunks.cpp
@@ -792,7 +792,14 @@ bool ThumbThunk::getMayUseShortThunk() {
if (!mayUseShortThunk)
return false;
uint64_t s = getARMThunkDestVA(ctx, destination);
- if ((s & 1) == 0 || !ctx.arg.armJ1J2BranchEncoding) {
+ // To use a short thunk the destination must be Thumb and the target must
+ // have the wide branch instruction B.w. This instruction is included when
+ // Thumb 2 is present, or in v8-M (and above) baseline architectures.
+ // armJ1J2BranchEncoding is available in all architectures with a profile and
+ // the one v6 CPU that implements Thumb 2 (Arm1156t2-s).
+ // Movt and Movw instructions require Thumb 2 or v8-M baseline.
+ if ((s & 1) == 0 || !ctx.arg.armJ1J2BranchEncoding ||
+ !ctx.arg.armHasMovtMovw) {
mayUseShortThunk = false;
addLongMapSyms();
return false;
diff --git a/lld/test/ELF/arm-thumb-thunk-v6m-noshort.s b/lld/test/ELF/arm-thumb-thunk-v6m-noshort.s
new file mode 100644
index 00000000000000..6b68b7bde67d03
--- /dev/null
+++ b/lld/test/ELF/arm-thumb-thunk-v6m-noshort.s
@@ -0,0 +1,57 @@
+// REQUIRES: arm
+// RUN: rm -rf %t && split-file %s %t && cd %t
+// RUN: llvm-mc -filetype=obj -arm-add-build-attributes -triple=armv6m-none-eabi asm -o v6m.o
+// RUN: ld.lld --script=lds v6m.o -o v6m
+// RUN: llvm-objdump --no-print-imm-hex --no-show-raw-insn -d v6m --triple=armv6m-none-eabi
+
+// RUN: llvm-mc -filetype=obj -arm-add-build-attributes -triple=armv8m.base-none-eabi asm -o v8m.o
+// RUN: ld.lld --script=lds v8m.o -o v8m
+// RUN: llvm-objdump --no-print-imm-hex --no-show-raw-insn -d v8m --triple=armv8m.base-none-eabi | FileCheck --check-prefix=CHECKV8BASE %s
+
+/// Test that short thunks are not generated for v6-m as this architecture
+/// does not have the B.w instruction.
+
+//--- asm
+ .syntax unified
+
+ .section .text_low, "ax", %progbits
+ .thumb
+ .type _start, %function
+ .balign 4
+ .globl _start
+_start:
+ bl far
+ .space 0x1000 - (. - _start)
+
+/// Thunks will be inserted here. They are in short thunk range for a B.w
+/// instruction. Expect v6-M to use a long thunk as v6-M does not have B.w.
+/// Expect v8-m.base to use a short thunk as despite not having Thumb 2 it
+/// does have B.w.
+
+// CHECK-LABEL: <__Thumbv6MABSLongThunk_far>:
+// CHECK-NEXT: 2000: push {r0, r1}
+// CHECK-NEXT: ldr r0, [pc, #4]
+// CHECK-NEXT: str r0, [sp, #4]
+// CHECK-NEXT: pop {r0, pc}
+// CHECK-NEXT: bo01 20 00 01 .word 0x01002001
+
+// CHECKV8BASE-LABEL: <__Thumbv7ABSLongThunk_far>:
+// CHECKV8BASE-NEXT: 2000: b.w 0x1002000 <far>
+
+ .section .text_high, "ax", %progbits
+ .globl far
+ .type far, %function
+ .balign 4
+far:
+ bx lr
+
+//--- lds
+
+PHDRS {
+ low PT_LOAD FLAGS(0x1 | 0x4);
+ high PT_LOAD FLAGS(0x1 | 0x4);
+}
+SECTIONS {
+ .text_low 0x1000 : { *(.text_low) }
+ .text_high 0x1002000 : { *(.text_high) }
+}
|
| @@ -0,0 +1,57 @@ | |||
| // REQUIRES: arm | |||
There was a problem hiding this comment.
The test passes even without the code change. Is that expected?
There was a problem hiding this comment.
Apologies it looks like I forgot to add FileCheck to the objdump for v6-m. Now done, which revealed a typo induced by fumbling a tmux switch buffer.
Tested that this now passes with the patch, and doesn't with an existing lld.
Also remove errant tmux switch buffer typo.
|
I think this is ready to look at again. I've fixed the test case so it will only pass with the short branches disabled. |
Thumb short thunks use the B.w instruction. This instruction is not present on Arm v6-m so we should prevent these targets from using short-thunks. We want to permit Arm v8-m.base targets to continue using short thunks as it does have the B.w instruction despite not implementing all of Thumb 2.
Add a check to see if the Movt and Movw instructions are present before enabling short thunks for Thumb. The v6-m architecture has J1J2BranchEncoding, but it does not have Movt and Movw, whereas v8-m.base has both.
The memory map and limited flash size of an Arm v6-m CPU makes a short thunk very unlikely in practice, but it is worth getting it right just in case.