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

Add support for leaf function frame pointer elimination #86652

Merged
merged 1 commit into from
Jul 1, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 20 additions & 10 deletions compiler/rustc_codegen_llvm/src/attributes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ use rustc_middle::ty::{self, TyCtxt};
use rustc_session::config::OptLevel;
use rustc_session::Session;
use rustc_target::spec::abi::Abi;
use rustc_target::spec::{SanitizerSet, StackProbeType};
use rustc_target::spec::{FramePointer, SanitizerSet, StackProbeType};

use crate::attributes;
use crate::llvm::AttributePlace::Function;
Expand Down Expand Up @@ -69,15 +69,25 @@ fn naked(val: &'ll Value, is_naked: bool) {
Attribute::Naked.toggle_llfn(Function, val, is_naked);
}

pub fn set_frame_pointer_elimination(cx: &CodegenCx<'ll, '_>, llfn: &'ll Value) {
if cx.sess().must_not_eliminate_frame_pointers() {
llvm::AddFunctionAttrStringValue(
llfn,
llvm::AttributePlace::Function,
cstr!("frame-pointer"),
cstr!("all"),
);
pub fn set_frame_pointer_type(cx: &CodegenCx<'ll, '_>, llfn: &'ll Value) {
let mut fp = cx.sess().target.frame_pointer;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

...but in this place, the behavior is now to take the target frame pointer first...

// "mcount" function relies on stack pointer.
// See <https://sourceware.org/binutils/docs/gprof/Implementation.html>.
if cx.sess().instrument_mcount() || matches!(cx.sess().opts.cg.force_frame_pointers, Some(true))
{
fp = FramePointer::Always;
Comment on lines +76 to +78
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

...and only consult the CLI's opinion if it demands them.

}
let attr_value = match fp {
FramePointer::Always => cstr!("all"),
FramePointer::NonLeaf => cstr!("non-leaf"),
FramePointer::MayOmit => return,
};
llvm::AddFunctionAttrStringValue(
llfn,
llvm::AttributePlace::Function,
cstr!("frame-pointer"),
attr_value,
);
}

/// Tell LLVM what instrument function to insert.
Expand Down Expand Up @@ -254,7 +264,7 @@ pub fn from_fn_attrs(cx: &CodegenCx<'ll, 'tcx>, llfn: &'ll Value, instance: ty::
}

// FIXME: none of these three functions interact with source level attributes.
set_frame_pointer_elimination(cx, llfn);
set_frame_pointer_type(cx, llfn);
set_instrument_function(cx, llfn);
set_probestack(cx, llfn);

Expand Down
4 changes: 2 additions & 2 deletions compiler/rustc_codegen_llvm/src/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -410,8 +410,8 @@ impl MiscMethods<'tcx> for CodegenCx<'ll, 'tcx> {
&self.used_statics
}

fn set_frame_pointer_elimination(&self, llfn: &'ll Value) {
attributes::set_frame_pointer_elimination(self, llfn)
fn set_frame_pointer_type(&self, llfn: &'ll Value) {
attributes::set_frame_pointer_type(self, llfn)
}

fn apply_target_cpu_attr(&self, llfn: &'ll Value) {
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_codegen_llvm/src/intrinsic.rs
Original file line number Diff line number Diff line change
Expand Up @@ -674,7 +674,7 @@ fn gen_fn<'ll, 'tcx>(
) -> &'ll Value {
let fn_abi = FnAbi::of_fn_ptr(cx, rust_fn_sig, &[]);
let llfn = cx.declare_fn(name, &fn_abi);
cx.set_frame_pointer_elimination(llfn);
cx.set_frame_pointer_type(llfn);
cx.apply_target_cpu_attr(llfn);
// FIXME(eddyb) find a nicer way to do this.
unsafe { llvm::LLVMRustSetLinkage(llfn, llvm::Linkage::InternalLinkage) };
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_codegen_ssa/src/base.rs
Original file line number Diff line number Diff line change
Expand Up @@ -406,7 +406,7 @@ pub fn maybe_create_entry_wrapper<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>>(
};

// `main` should respect same config for frame pointer elimination as rest of code
cx.set_frame_pointer_elimination(llfn);
cx.set_frame_pointer_type(llfn);
cx.apply_target_cpu_attr(llfn);

let llbb = Bx::append_block(&cx, llfn, "top");
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_codegen_ssa/src/traits/misc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ pub trait MiscMethods<'tcx>: BackendTypes {
fn sess(&self) -> &Session;
fn codegen_unit(&self) -> &'tcx CodegenUnit<'tcx>;
fn used_statics(&self) -> &RefCell<Vec<Self::Value>>;
fn set_frame_pointer_elimination(&self, llfn: Self::Function);
fn set_frame_pointer_type(&self, llfn: Self::Function);
fn apply_target_cpu_attr(&self, llfn: Self::Function);
fn create_used_variable(&self);
/// Declares the extern "C" main function for the entry point. Returns None if the symbol already exists.
Expand Down
12 changes: 0 additions & 12 deletions compiler/rustc_session/src/session.rs
Original file line number Diff line number Diff line change
Expand Up @@ -792,18 +792,6 @@ impl Session {
!self.target.is_like_windows && !self.target.is_like_osx
}

pub fn must_not_eliminate_frame_pointers(&self) -> bool {
// "mcount" function relies on stack pointer.
// See <https://sourceware.org/binutils/docs/gprof/Implementation.html>.
if self.instrument_mcount() {
true
} else if let Some(x) = self.opts.cg.force_frame_pointers {
x
} else {
!self.target.eliminate_frame_pointer
}
}

Comment on lines -795 to -806
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In this PR, the behavior here seems to have been reversed: previously, the target decided the default frame pointer disposition last, and the CLI argument took precedence...

pub fn must_emit_unwind_tables(&self) -> bool {
// This is used to control the emission of the `uwtable` attribute on
// LLVM functions.
Expand Down
8 changes: 6 additions & 2 deletions compiler/rustc_target/src/spec/aarch64_apple_darwin.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use crate::spec::{LinkerFlavor, SanitizerSet, Target, TargetOptions};
use crate::spec::{FramePointer, LinkerFlavor, SanitizerSet, Target, TargetOptions};

pub fn target() -> Target {
let mut base = super::apple_base::opts("macos");
Expand All @@ -20,6 +20,10 @@ pub fn target() -> Target {
pointer_width: 64,
data_layout: "e-m:o-i64:64-i128:128-n32:64-S128".to_string(),
arch: arch.to_string(),
options: TargetOptions { mcount: "\u{1}mcount".to_string(), ..base },
options: TargetOptions {
mcount: "\u{1}mcount".to_string(),
frame_pointer: FramePointer::NonLeaf,
..base
},
}
}
3 changes: 2 additions & 1 deletion compiler/rustc_target/src/spec/aarch64_apple_ios.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
use super::apple_sdk_base::{opts, Arch};
use crate::spec::{Target, TargetOptions};
use crate::spec::{FramePointer, Target, TargetOptions};

pub fn target() -> Target {
let base = opts("ios", Arch::Arm64);
Expand All @@ -13,6 +13,7 @@ pub fn target() -> Target {
max_atomic_width: Some(128),
unsupported_abis: super::arm_base::unsupported_abis(),
forces_embed_bitcode: true,
frame_pointer: FramePointer::NonLeaf,
// Taken from a clang build on Xcode 11.4.1.
// These arguments are not actually invoked - they just have
// to look right to pass App Store validation.
Expand Down
3 changes: 2 additions & 1 deletion compiler/rustc_target/src/spec/aarch64_apple_ios_macabi.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
use super::apple_sdk_base::{opts, Arch};
use crate::spec::{Target, TargetOptions};
use crate::spec::{FramePointer, Target, TargetOptions};

pub fn target() -> Target {
let base = opts("ios", Arch::Arm64_macabi);
Expand All @@ -13,6 +13,7 @@ pub fn target() -> Target {
max_atomic_width: Some(128),
unsupported_abis: super::arm_base::unsupported_abis(),
forces_embed_bitcode: true,
frame_pointer: FramePointer::NonLeaf,
// Taken from a clang build on Xcode 11.4.1.
// These arguments are not actually invoked - they just have
// to look right to pass App Store validation.
Expand Down
3 changes: 2 additions & 1 deletion compiler/rustc_target/src/spec/aarch64_apple_ios_sim.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
use super::apple_sdk_base::{opts, Arch};
use crate::spec::{Target, TargetOptions};
use crate::spec::{FramePointer, Target, TargetOptions};

pub fn target() -> Target {
let base = opts("ios", Arch::Arm64_sim);
Expand All @@ -21,6 +21,7 @@ pub fn target() -> Target {
max_atomic_width: Some(128),
unsupported_abis: super::arm_base::unsupported_abis(),
forces_embed_bitcode: true,
frame_pointer: FramePointer::NonLeaf,
// Taken from a clang build on Xcode 11.4.1.
// These arguments are not actually invoked - they just have
// to look right to pass App Store validation.
Expand Down
3 changes: 2 additions & 1 deletion compiler/rustc_target/src/spec/aarch64_apple_tvos.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
use super::apple_sdk_base::{opts, Arch};
use crate::spec::{Target, TargetOptions};
use crate::spec::{FramePointer, Target, TargetOptions};

pub fn target() -> Target {
let base = opts("tvos", Arch::Arm64);
Expand All @@ -13,6 +13,7 @@ pub fn target() -> Target {
max_atomic_width: Some(128),
unsupported_abis: super::arm_base::unsupported_abis(),
forces_embed_bitcode: true,
frame_pointer: FramePointer::NonLeaf,
..base
},
}
Expand Down
4 changes: 2 additions & 2 deletions compiler/rustc_target/src/spec/apple_base.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
use std::env;

use crate::spec::{SplitDebuginfo, TargetOptions};
use crate::spec::{FramePointer, SplitDebuginfo, TargetOptions};

pub fn opts(os: &str) -> TargetOptions {
// ELF TLS is only available in macOS 10.7+. If you try to compile for 10.6
Expand All @@ -27,7 +27,7 @@ pub fn opts(os: &str) -> TargetOptions {
families: vec!["unix".to_string()],
is_like_osx: true,
dwarf_version: Some(2),
eliminate_frame_pointer: false,
frame_pointer: FramePointer::Always,
has_rpath: true,
dll_suffix: ".dylib".to_string(),
archive_format: "darwin".to_string(),
Expand Down
4 changes: 2 additions & 2 deletions compiler/rustc_target/src/spec/freebsd_base.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use crate::spec::{RelroLevel, TargetOptions};
use crate::spec::{FramePointer, RelroLevel, TargetOptions};

pub fn opts() -> TargetOptions {
TargetOptions {
Expand All @@ -8,7 +8,7 @@ pub fn opts() -> TargetOptions {
families: vec!["unix".to_string()],
has_rpath: true,
position_independent_executables: true,
eliminate_frame_pointer: false, // FIXME 43575
frame_pointer: FramePointer::Always, // FIXME 43575: should be MayOmit...
relro_level: RelroLevel::Full,
abi_return_struct_as_int: true,
dwarf_version: Some(2),
Expand Down
4 changes: 2 additions & 2 deletions compiler/rustc_target/src/spec/i686_apple_darwin.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use crate::spec::{LinkerFlavor, StackProbeType, Target, TargetOptions};
use crate::spec::{FramePointer, LinkerFlavor, StackProbeType, Target, TargetOptions};

pub fn target() -> Target {
let mut base = super::apple_base::opts("macos");
Expand All @@ -8,7 +8,7 @@ pub fn target() -> Target {
base.link_env_remove.extend(super::apple_base::macos_link_env_remove());
// don't use probe-stack=inline-asm until rust#83139 and rust#84667 are resolved
base.stack_probes = StackProbeType::Call;
base.eliminate_frame_pointer = false;
base.frame_pointer = FramePointer::Always;

// Clang automatically chooses a more specific target based on
// MACOSX_DEPLOYMENT_TARGET. To enable cross-language LTO to work
Expand Down
4 changes: 2 additions & 2 deletions compiler/rustc_target/src/spec/i686_pc_windows_gnu.rs
Original file line number Diff line number Diff line change
@@ -1,12 +1,12 @@
use crate::spec::{LinkerFlavor, LldFlavor, Target};
use crate::spec::{FramePointer, LinkerFlavor, LldFlavor, Target};

pub fn target() -> Target {
let mut base = super::windows_gnu_base::opts();
base.cpu = "pentium4".to_string();
base.pre_link_args
.insert(LinkerFlavor::Lld(LldFlavor::Ld), vec!["-m".to_string(), "i386pe".to_string()]);
base.max_atomic_width = Some(64);
base.eliminate_frame_pointer = false; // Required for backtraces
base.frame_pointer = FramePointer::Always; // Required for backtraces
base.linker = Some("i686-w64-mingw32-gcc".to_string());

// Mark all dynamic libraries and executables as compatible with the larger 4GiB address
Expand Down
4 changes: 2 additions & 2 deletions compiler/rustc_target/src/spec/i686_unknown_linux_musl.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use crate::spec::{LinkerFlavor, StackProbeType, Target};
use crate::spec::{FramePointer, LinkerFlavor, StackProbeType, Target};

pub fn target() -> Target {
let mut base = super::linux_musl_base::opts();
Expand All @@ -21,7 +21,7 @@ pub fn target() -> Target {
//
// This may or may not be related to this bug:
// https://llvm.org/bugs/show_bug.cgi?id=30879
base.eliminate_frame_pointer = false;
base.frame_pointer = FramePointer::Always;

Target {
llvm_target: "i686-unknown-linux-musl".to_string(),
Expand Down
4 changes: 2 additions & 2 deletions compiler/rustc_target/src/spec/i686_uwp_windows_gnu.rs
Original file line number Diff line number Diff line change
@@ -1,12 +1,12 @@
use crate::spec::{LinkerFlavor, LldFlavor, Target};
use crate::spec::{FramePointer, LinkerFlavor, LldFlavor, Target};

pub fn target() -> Target {
let mut base = super::windows_uwp_gnu_base::opts();
base.cpu = "pentium4".to_string();
base.pre_link_args
.insert(LinkerFlavor::Lld(LldFlavor::Ld), vec!["-m".to_string(), "i386pe".to_string()]);
base.max_atomic_width = Some(64);
base.eliminate_frame_pointer = false; // Required for backtraces
base.frame_pointer = FramePointer::Always; // Required for backtraces

// Mark all dynamic libraries and executables as compatible with the larger 4GiB address
// space available to x86 Windows binaries on x86_64.
Expand Down
4 changes: 2 additions & 2 deletions compiler/rustc_target/src/spec/illumos_base.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use crate::spec::{LinkArgs, LinkerFlavor, TargetOptions};
use crate::spec::{FramePointer, LinkArgs, LinkerFlavor, TargetOptions};
use std::default::Default;

pub fn opts() -> TargetOptions {
Expand Down Expand Up @@ -35,7 +35,7 @@ pub fn opts() -> TargetOptions {
is_like_solaris: true,
linker_is_gnu: false,
limit_rdylib_exports: false, // Linker doesn't support this
eliminate_frame_pointer: false,
frame_pointer: FramePointer::Always,
eh_frame_header: false,
late_link_args,

Expand Down
5 changes: 3 additions & 2 deletions compiler/rustc_target/src/spec/linux_kernel_base.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
use crate::spec::{PanicStrategy, RelocModel, RelroLevel, StackProbeType, TargetOptions};
use crate::spec::TargetOptions;
use crate::spec::{FramePointer, PanicStrategy, RelocModel, RelroLevel, StackProbeType};

pub fn opts() -> TargetOptions {
TargetOptions {
Expand All @@ -7,7 +8,7 @@ pub fn opts() -> TargetOptions {
panic_strategy: PanicStrategy::Abort,
// don't use probe-stack=inline-asm until rust#83139 and rust#84667 are resolved
stack_probes: StackProbeType::Call,
eliminate_frame_pointer: false,
frame_pointer: FramePointer::Always,
position_independent_executables: true,
needs_plt: true,
relro_level: RelroLevel::Full,
Expand Down
Loading