[RISC-V] Do not force frame pointers
We have been seeing some very inefficient code that went away when using
`-Cforce-frame-pointers=no`. For instance `core::ptr::drop_in_place` at
`-Oz` was compiled into a function which consisted entirely of saving
registers to the stack, then using the frame pointer to restore the same
registers (without any instructions between the prolog and epilog).
The RISC-V LLVM backend supports frame pointer elimination, so it makes
sense to allow this to happen when using Rust. It's not clear to me that
frame pointers have ever been required in the general case.
In rust-lang/rust#61675 it was pointed out that this made reassembling
stack traces easier, which is true, but there is a code generation
option for forcing frame pointers, and I feel the default should not be
to require frame pointers, given it demonstrably makes code size worse
(around 10% in some embedded applications).
The kinds of targets mentioned in rust-lang/rust#61675 are popular, but
should not dictate that code generation should be worse for all RISC-V
targets, especially as there is a way to use CFI information to
reconstruct the stack when the frame pointer is eliminated. It is also
a misconception that `fp` is always used for the frame pointer. `fp` is
an ABI name for `x8` (aka `s0`), and if no frame pointer is required,
`x8` may be used for other callee-saved values.
---
I am partly posting this to get feedback from @fintelia who introduced the change to require frame pointers, and @hanna-kruppe who had issues with the original PR. I would understand if we wanted to remove this setting on only a subset of RISC-V targets, but my preference would be to remove this setting everywhere.
There are more details on the code size savings seen in Tock here: https://github.com/tock/tock/pull/1660
If config.toml `profiler = false`, the test/mir-opt/instrument_coverage
test is ignored. Otherwise, this patch ensures the profiler_runtime is
loaded when -Zinstrument-coverage is enabled. Confirmed that this works
for MacOS.
Update LLVM submodule
Only includes one commit:
- [D80759](https://reviews.llvm.org/D80759): Fix FastISel dropping srcloc metadata from InlineAsm
Fixes#40555
Rollup of 8 pull requests
Successful merges:
- #73237 (Check for overflow in DroplessArena and align returned memory)
- #73339 (Don't run generator transform when there's a TyErr)
- #73372 (Re-order correctly the sections in the sidebar)
- #73373 (Use track caller for bug! macro)
- #73380 (Add more info to `x.py build --help` on default value for `-j JOBS`.)
- #73381 (Fix typo in docs of std::mem)
- #73389 (Use `Ipv4Addr::from<[u8; 4]>` when possible)
- #73400 (Fix forge-platform-support URL)
Failed merges:
r? @ghost
Re-order correctly the sections in the sidebar
Before that, "trait implementations" and "implementors" titles in the sidebar were before "methods" for example. Which wasn't logical considering that the two sections come after in the "content".
r? @kinnison
Don't run generator transform when there's a TyErr
Not sure if this might cause any problems later on, but we shouldn't be hitting codegen or const eval for the produced MIR anyways, so it should be fine.
cc https://github.com/rust-lang/rust/issues/72685#issuecomment-643749020
Check for overflow in DroplessArena and align returned memory
* Check for overflow when calculating the slice start & end position.
* Align the pointer obtained from the allocator, ensuring that it
satisfies user requested alignment (the allocator is only asked for
layout compatible with u8 slice).
* Remove an incorrect assertion from DroplessArena::align.
* Avoid forming references to an uninitialized memory in DroplessArena.
Helps with #73007, #72624.
Avoid prematurely recording toolstates
When we're running with dry_run enabled (i.e. all builds do this initially), we're
guaranteed to save of a toolstate of TestFail for tools that aren't tested. In practice,
we do test tools as well, so for those tools we would initially record them as being
TestPass, and then later on re-record the correct state after actually testing them.
However, this would not work well if the build failed for whatever reason (e.g. panicking
in bootstrap, or as was the case in #73097, clippy failing to test successfully), we would
just go on believing that things passed when they in practice did not.
This commit also adjusts saving toolstate to never record clippy explicitly (otherwise, it
would be recorded when building it); eventually that'll likely move to other tools as well
but not yet. This is deemed simpler than checking everywhere we generically save
toolstate.
We also move clippy out of the "toolstate" no-fail-fast build into a separate x.py
invocation; this should no longer be technically required but provides the nice state of
letting us check toolstate for all tools and only then check clippy (giving full results
on every build).
r? @oli-obk
Supercedes #73275, also fixes#73274
