Load string indices with inline asm to save space.
~~Requires #878 because asm!(... sym ..) breaks if the symbol name has quote characters.~~ not anymore yay
Defmt string indices are 16 bits, which can be loaded with a single movw.
However, the compiler doesn't know that, so it generates movw+movt or ldr rX, [pc, #offs]
because it sees we're loading an address of a symbol, which could be any 32bit value.
This wastes space, so we load the value with asm manually to avoid this.
#[inline(never)]
unsafe fn testfoobar() {
info!("foo");
}
before:
00027180 <_ZN11application10testfoobar17h819f2778e910636eE>:
27180: b580 push {r7, lr}
27182: 466f mov r7, sp
27184: f000 f954 bl 0x27430 <_defmt_acquire> @ imm = #0x2a8
27188: 4803 ldr r0, [pc, #0xc] @ 0x27198 <_ZN11application10testfoobar17h819f2778e910636eE+0x18>
2718a: f000 fa35 bl 0x275f8 <_ZN5defmt6export6header17hd841c3329e459a6fE> @ imm = #0x46a
2718e: e8bd 4080 pop.w {r7, lr}
27192: f000 b95b b.w 0x2744c <_defmt_release> @ imm = #0x2b6
27196: bf00 nop
27198: 03 00 00 00 .word 0x00000003
after:
00027180 <_ZN11application10testfoobar17hc6c595839020df66E>:
27180: b580 push {r7, lr}
27182: 466f mov r7, sp
27184: f000 f94e bl 0x27424 <_defmt_acquire> @ imm = #0x29c
27188: f240 0003 movw r0, #0x3
2718c: f000 fa34 bl 0x275f8 <_ZN5defmt6export6header17h79a831a74481cee1E> @ imm = #0x468
27190: e8bd 4080 pop.w {r7, lr}
27194: f000 b954 b.w 0x27440 <_defmt_release> @ imm = #0x2a8
Thanks for this PR - it looks neat. Once we've resolve the question about hex-encoding symbols, I would support merging this.
(This isn't a breaking change, we just want cargo server-checks to stop complaining about a PR that was committed some time ago)
great news: https://github.com/llvm/llvm-project/pull/159420 fixes using quotes in asm!(sym). so #878 is not needed anymore.
I assume it'll land in Rust 1.91. What do we do?
- Bump MSRV to Rust 1.91+?
- Check rust version in
build.rsand do the optimization only in Rust 1.91+?
MSRV moving is hard when we have customers on long term support toolchains. A build.rs version switch, or a feature flag, would be fine.
I wonder if this is related to the quote escaping issue in LLVM 21.
A build.rs version switch, or a feature flag, would be fine.
Implemented a version switch. I wouldn't do a feature flag, it's not very discoverable and it's nice you get the optimization without having to opt-in.
We'll have to wait until the llvm fix lands in nightly/beta to merge this.
I wonder if this is related to the quote escaping issue in LLVM 21.
yep! basically:
- old LLVM would never unescape, causing the bug in
asm!(sym). - this PR changed it to always unescape, which fixed
asm!(sym)but broke#[export_name], causing https://github.com/rust-lang/rust/issues/146065 . - this PR changes it to unescape in the right places only, so finally both
asm!(sym)and#[export_name]work.
This looks good to me and I'm inclined to merge, but I'd like to do some code size and compile time comparisons first.