Binary size regression on msp430 due to #98582 #99685
Comments
cr1901
added
C-bug
rustbot
added
regression-from-stable-to-nightly
apiraino
removed
the
I-prioritize
oli-obk
removed
the
regression-from-stable-to-nightly
|
can you check whether #99806 does not have the same issue? There's a try build that you can download at 2a2d919ccd8dc0e7f8c61e9e8d07f865fb05e842 |
|
@oli-obk I wasn't sure how to download the try build, so I checked out the PR, compiled it locally, and then compared it to a recent commit (48de123) I also compiled: Short version, assuming that commit I checked is close enough, looks like #99806 does not have the same issue and is good to go :).
|
You can use rustup-toolchain-install-master with |
|
Thanks for checking! I'm going to go ahead and close this issue then. |
|
@lqd I will try I'm pretty sure I've seen this before, but have since forgotten how to solve it (the MCVE requires |
|
IIRC you can use -c when you initially install the try build artifacts. |
…estebank Allow patterns to constrain the hidden type of opaque types fixes rust-lang#96572 reverts a revert as original PR was a perf regression that was fixed by reverting it: rust-lang#99368 (comment)) TODO: * check if rust-lang#99685 is avoided
Okay, just to make crystal clear, the try build at 2a2d919c is identical to the results I have above (i.e. no problems). Didn't think it would be different, but no harm in trying since I have all the toolchain stuff already installed. Thanks for the help :D! |
Allow patterns to constrain the hidden type of opaque types fixes #96572 reverts a revert as original PR was a perf regression that was fixed by reverting it: rust-lang/rust#99368 (comment)) TODO: * check if rust-lang/rust#99685 is avoided
Allow patterns to constrain the hidden type of opaque types fixes #96572 reverts a revert as original PR was a perf regression that was fixed by reverting it: rust-lang/rust#99368 (comment)) TODO: * check if rust-lang/rust#99685 is avoided
I recently had CI start failing for some firmware I use to make sure Rust still compiles msp430 code correctly. It appears that as of #98582, Rust has stopped optimizing out dead panic code; there is no I/O device to send the panic strings to, and so panic strings- and setting up arguments to access panic strings- should be considered dead code.
Normally, the firmware size should take 1994 (1992+2) bytes:
However, the firmware size had gone up 100 bytes (2048+46 = 2094- 5% regression) before #98582 was reverted by #99495:
I've created an MVCE from the failing CI to illustrate:
Code
Instructions
Make sure the
msp430-elf-gcctoolchain is installed. Optionally installjustfor convenience.git clone https://github.com/cr1901/msp430-size. Use commit e44cf66 specifically.Compile
rustcand add your newrustctorustupusingrustup toolchain link override-name /path/to/override. Use this override for bothrustccommits in the next step.Run the below command twice (using different
targetdirs):The above command needs to be run with two different compilers:
Extra compiler artifacts can be generated like so:
Expected Results
For both Rust compiler commits, I expected to see the following output from
msp430-elf-size, with no panic strings emitted:Instead, the compiler which had PR #98582 had a different size, and a panic string for
Option::unwrap()failure was emitted into the binary:Regression Commits
Based on
git bisect, this regression was introduced by PR #98582. Specifically 728c7e8 introduced the regression. This regression was fixed by #99495. However, @oli-obk asked me to open an issue anyway.@rustbot modify labels: +regression-from-stable-to-nightly -regression-untriaged
Other Context
Although I don't have numbers handy, this regression also affects
libcores compiled with thepanic_immediate_abortfeature for less marked size regression.The text was updated successfully, but these errors were encountered: