Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

28x binary size regression in recent nightly #85519

Closed
jonas-schievink opened this issue May 20, 2021 · 9 comments · Fixed by #85531
Closed

28x binary size regression in recent nightly #85519

jonas-schievink opened this issue May 20, 2021 · 9 comments · Fixed by #85531
Labels
A-linkage Area: linking into static, shared libraries and binaries C-bug Category: This is a bug. I-heavy Issue: Problems and improvements with respect to binary size of generated code. regression-from-stable-to-nightly Performance or correctness regression from stable to nightly. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@jonas-schievink
Copy link
Contributor

There seems to be a significant regression in binary size in the most recent nightly:

nightly-2021-05-19

$ size target/thumbv6m-none-eabi/debug/hello
   text	   data	    bss	    dec	    hex	filename
   6056	     48	   1032	   7136	   1be0	target/thumbv6m-none-eabi/debug/hello

nightly-2021-05-20

$ size target/thumbv6m-none-eabi/debug/hello
   text	   data	    bss	    dec	    hex	filename
 201372	     48	   1032	 202452	  316d4	target/thumbv6m-none-eabi/debug/hello

Steps to reproduce:

Commits in that range: 4e3e6db...f94942d

#85274 looks suspicious (cc @luqmana)

@jonas-schievink jonas-schievink added A-linkage Area: linking into static, shared libraries and binaries regression-from-stable-to-nightly Performance or correctness regression from stable to nightly. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. C-bug Category: This is a bug. I-heavy Issue: Problems and improvements with respect to binary size of generated code. labels May 20, 2021
@rustbot rustbot added the I-prioritize Issue: Indicates that prioritization has been requested for this issue. label May 20, 2021
@jamesmunns
Copy link
Member

jamesmunns commented May 20, 2021

This seems to have been noticed here as well: rust-osdev/bootloader#168

Edit: Fixed link to rust-osdev repo instead of this issue

Edit 2: This also indicates you can work-around the regression by manually passing --gc-sections as a linker flag.

@jamesmunns
Copy link
Member

Also as a note, @japaric tested this with using arm-none-eabi-ld as the linker instead of the default rust-lld, and this issue remained (referring to #85274). He can confirm whether he set the "linker flavor" or similar as well.

@Dirbaio
Copy link
Contributor

Dirbaio commented May 20, 2021

it's very likely #85274, since adding -C link-arg=--gc-sections to rustflags fixes the bloat.

@nagisa
Copy link
Member

nagisa commented May 20, 2021

Is the linker for this target guaranteed to be gnu ld? Doesn't sound like something that we can claim, can we? It does look like we default to lld.

@jonas-schievink
Copy link
Contributor Author

We do default to LLD, which can also garbage-collect unused sections

@nagisa
Copy link
Member

nagisa commented May 20, 2021

Yeah, I was just wondering if we should expand our logic around when we specify the --gc-sections flag or just say that the targets use a gnu-ld-like linker.

@jonas-schievink
Copy link
Contributor Author

jonas-schievink commented May 20, 2021

Yeah, in practice I don't think anyone uses a non-GNU-compatible linker there

@petrochenkov
Copy link
Contributor

It may make sense to set the default for linker_is_gnu to true because ld-style linkers are usually gnu, and it's easier to explicitly list exceptions than to not forget setting it to true for all random embedded targets.

@petrochenkov
Copy link
Contributor

If the default is not flipped then compiler\rustc_target\src\spec\thumb_base.rs needs to add linker_is_gnu: true + other target specs should probably be audited as well.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-linkage Area: linking into static, shared libraries and binaries C-bug Category: This is a bug. I-heavy Issue: Problems and improvements with respect to binary size of generated code. regression-from-stable-to-nightly Performance or correctness regression from stable to nightly. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
7 participants