defmt icon indicating copy to clipboard operation
defmt copied to clipboard

Load string indices with inline asm to save space.

Open Dirbaio opened this issue 1 year ago • 6 comments

~~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

Dirbaio avatar Oct 28 '24 18:10 Dirbaio

Thanks for this PR - it looks neat. Once we've resolve the question about hex-encoding symbols, I would support merging this.

jonathanpallant avatar Nov 04 '24 13:11 jonathanpallant

(This isn't a breaking change, we just want cargo server-checks to stop complaining about a PR that was committed some time ago)

jonathanpallant avatar Nov 06 '24 14:11 jonathanpallant

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.rs and do the optimization only in Rust 1.91+?

Dirbaio avatar Sep 19 '25 14:09 Dirbaio

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.

jonathanpallant avatar Sep 19 '25 15:09 jonathanpallant

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.

Dirbaio avatar Sep 19 '25 17:09 Dirbaio

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.

jonathanpallant avatar Sep 25 '25 10:09 jonathanpallant