[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. This commit does ensure that the standard library is built with unwind tables, so that users do not need to rebuild the standard library in order to get a backtrace that includes standard library calls (which is the original reason for forcing frame pointers).
This commit is contained in:
parent
91fb72a8a9
commit
3da3d15f9f
6 changed files with 10 additions and 5 deletions
|
|
@ -244,6 +244,16 @@ pub fn std_cargo(builder: &Builder<'_>, target: Interned<String>, stage: u32, ca
|
|||
if stage >= 1 {
|
||||
cargo.rustflag("-Cembed-bitcode=yes");
|
||||
}
|
||||
|
||||
// By default, rustc does not include unwind tables unless they are required
|
||||
// for a particular target. They are not required by RISC-V targets, but
|
||||
// compiling the standard library with them means that users can get
|
||||
// backtraces without having to recompile the standard library themselves.
|
||||
//
|
||||
// This choice was discussed in https://github.com/rust-lang/rust/pull/69890
|
||||
if target.contains("riscv") {
|
||||
cargo.rustflag("-Cforce-unwind-tables=yes");
|
||||
}
|
||||
}
|
||||
|
||||
#[derive(Debug, Copy, Clone, PartialEq, Eq, Hash)]
|
||||
|
|
|
|||
|
|
@ -25,7 +25,6 @@ pub fn target() -> TargetResult {
|
|||
relocation_model: RelocModel::Static,
|
||||
emit_debug_gdb_scripts: false,
|
||||
abi_blacklist: super::riscv_base::abi_blacklist(),
|
||||
eliminate_frame_pointer: false,
|
||||
..Default::default()
|
||||
},
|
||||
})
|
||||
|
|
|
|||
|
|
@ -25,7 +25,6 @@ pub fn target() -> TargetResult {
|
|||
relocation_model: RelocModel::Static,
|
||||
emit_debug_gdb_scripts: false,
|
||||
abi_blacklist: super::riscv_base::abi_blacklist(),
|
||||
eliminate_frame_pointer: false,
|
||||
..Default::default()
|
||||
},
|
||||
})
|
||||
|
|
|
|||
|
|
@ -25,7 +25,6 @@ pub fn target() -> TargetResult {
|
|||
relocation_model: RelocModel::Static,
|
||||
emit_debug_gdb_scripts: false,
|
||||
abi_blacklist: super::riscv_base::abi_blacklist(),
|
||||
eliminate_frame_pointer: false,
|
||||
..Default::default()
|
||||
},
|
||||
})
|
||||
|
|
|
|||
|
|
@ -26,7 +26,6 @@ pub fn target() -> TargetResult {
|
|||
code_model: Some(CodeModel::Medium),
|
||||
emit_debug_gdb_scripts: false,
|
||||
abi_blacklist: super::riscv_base::abi_blacklist(),
|
||||
eliminate_frame_pointer: false,
|
||||
..Default::default()
|
||||
},
|
||||
})
|
||||
|
|
|
|||
|
|
@ -26,7 +26,6 @@ pub fn target() -> TargetResult {
|
|||
code_model: Some(CodeModel::Medium),
|
||||
emit_debug_gdb_scripts: false,
|
||||
abi_blacklist: super::riscv_base::abi_blacklist(),
|
||||
eliminate_frame_pointer: false,
|
||||
..Default::default()
|
||||
},
|
||||
})
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue