about summary refs log tree commit diff
diff options
context:
space:
mode:
authorSam Elliott <selliott@lowrisc.org>2020-05-30 18:24:19 +0100
committerSam Elliott <selliott@lowrisc.org>2020-05-30 18:24:19 +0100
commit3da3d15f9f150dafd7d635859795c4c714e7b7ad (patch)
tree3a05190a0d9068249309c8e200556d387b67a15c
parent91fb72a8a9f53de2bcc5638c1358fcb552dba8ce (diff)
downloadrust-3da3d15f9f150dafd7d635859795c4c714e7b7ad.tar.gz
rust-3da3d15f9f150dafd7d635859795c4c714e7b7ad.zip
[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).
-rw-r--r--src/bootstrap/compile.rs10
-rw-r--r--src/librustc_target/spec/riscv32i_unknown_none_elf.rs1
-rw-r--r--src/librustc_target/spec/riscv32imac_unknown_none_elf.rs1
-rw-r--r--src/librustc_target/spec/riscv32imc_unknown_none_elf.rs1
-rw-r--r--src/librustc_target/spec/riscv64gc_unknown_none_elf.rs1
-rw-r--r--src/librustc_target/spec/riscv64imac_unknown_none_elf.rs1
6 files changed, 10 insertions, 5 deletions
diff --git a/src/bootstrap/compile.rs b/src/bootstrap/compile.rs
index b3999118e3d..46c43be992d 100644
--- a/src/bootstrap/compile.rs
+++ b/src/bootstrap/compile.rs
@@ -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)]
diff --git a/src/librustc_target/spec/riscv32i_unknown_none_elf.rs b/src/librustc_target/spec/riscv32i_unknown_none_elf.rs
index aade1e70823..d7b3e7e1530 100644
--- a/src/librustc_target/spec/riscv32i_unknown_none_elf.rs
+++ b/src/librustc_target/spec/riscv32i_unknown_none_elf.rs
@@ -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()
         },
     })
diff --git a/src/librustc_target/spec/riscv32imac_unknown_none_elf.rs b/src/librustc_target/spec/riscv32imac_unknown_none_elf.rs
index e2990eeb826..b93b6fcf800 100644
--- a/src/librustc_target/spec/riscv32imac_unknown_none_elf.rs
+++ b/src/librustc_target/spec/riscv32imac_unknown_none_elf.rs
@@ -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()
         },
     })
diff --git a/src/librustc_target/spec/riscv32imc_unknown_none_elf.rs b/src/librustc_target/spec/riscv32imc_unknown_none_elf.rs
index 55a4d58dfcc..a16e7e31c66 100644
--- a/src/librustc_target/spec/riscv32imc_unknown_none_elf.rs
+++ b/src/librustc_target/spec/riscv32imc_unknown_none_elf.rs
@@ -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()
         },
     })
diff --git a/src/librustc_target/spec/riscv64gc_unknown_none_elf.rs b/src/librustc_target/spec/riscv64gc_unknown_none_elf.rs
index 7376a14e951..e5147a12ed3 100644
--- a/src/librustc_target/spec/riscv64gc_unknown_none_elf.rs
+++ b/src/librustc_target/spec/riscv64gc_unknown_none_elf.rs
@@ -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()
         },
     })
diff --git a/src/librustc_target/spec/riscv64imac_unknown_none_elf.rs b/src/librustc_target/spec/riscv64imac_unknown_none_elf.rs
index a3b0eb5334f..dc056b55b38 100644
--- a/src/librustc_target/spec/riscv64imac_unknown_none_elf.rs
+++ b/src/librustc_target/spec/riscv64imac_unknown_none_elf.rs
@@ -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()
         },
     })