From dcbdc8c19b4b1e581b8b83a513b11c4c4248d0fc Mon Sep 17 00:00:00 2001 From: Erik Desjardins Date: Thu, 17 Feb 2022 00:58:13 -0500 Subject: [PATCH 1/4] At opt-level=0, apply only ABI-affecting attributes to functions This should provide a small perf improvement for debug builds, and should more than cancel out the regression from adding noundef, which was only significant in debug builds. --- compiler/rustc_codegen_llvm/src/abi.rs | 93 +++++++++++--------- src/test/codegen/function-arguments-noopt.rs | 63 +++++++++++++ 2 files changed, 115 insertions(+), 41 deletions(-) create mode 100644 src/test/codegen/function-arguments-noopt.rs diff --git a/compiler/rustc_codegen_llvm/src/abi.rs b/compiler/rustc_codegen_llvm/src/abi.rs index 8a11e3e71bc81..a20801914adfa 100644 --- a/compiler/rustc_codegen_llvm/src/abi.rs +++ b/compiler/rustc_codegen_llvm/src/abi.rs @@ -13,6 +13,7 @@ use rustc_middle::bug; use rustc_middle::ty::layout::LayoutOf; pub use rustc_middle::ty::layout::{FAT_PTR_ADDR, FAT_PTR_EXTRA}; use rustc_middle::ty::Ty; +use rustc_session::config; use rustc_target::abi::call::ArgAbi; pub use rustc_target::abi::call::*; use rustc_target::abi::{self, HasDataLayout, Int}; @@ -20,27 +21,6 @@ pub use rustc_target::spec::abi::Abi; use libc::c_uint; -macro_rules! for_each_kind { - ($flags: ident, $f: ident, $($kind: ident),+) => ({ - $(if $flags.contains(ArgAttribute::$kind) { $f(llvm::Attribute::$kind) })+ - }) -} - -trait ArgAttributeExt { - fn for_each_kind(&self, f: F) - where - F: FnMut(llvm::Attribute); -} - -impl ArgAttributeExt for ArgAttribute { - fn for_each_kind(&self, mut f: F) - where - F: FnMut(llvm::Attribute), - { - for_each_kind!(self, f, NoAlias, NoCapture, NonNull, ReadOnly, InReg, NoUndef) - } -} - pub trait ArgAttributesExt { fn apply_attrs_to_llfn(&self, idx: AttributePlace, cx: &CodegenCx<'_, '_>, llfn: &Value); fn apply_attrs_to_callsite( @@ -58,10 +38,36 @@ fn should_use_mutable_noalias(cx: &CodegenCx<'_, '_>) -> bool { cx.tcx.sess.opts.debugging_opts.mutable_noalias.unwrap_or(true) } +const ABI_AFFECTING_ATTRIBUTES: [(ArgAttribute, llvm::Attribute); 1] = + [(ArgAttribute::InReg, llvm::Attribute::InReg)]; + +const OPTIMIZATION_ATTRIBUTES: [(ArgAttribute, llvm::Attribute); 5] = [ + (ArgAttribute::NoAlias, llvm::Attribute::NoAlias), + (ArgAttribute::NoCapture, llvm::Attribute::NoCapture), + (ArgAttribute::NonNull, llvm::Attribute::NonNull), + (ArgAttribute::ReadOnly, llvm::Attribute::ReadOnly), + (ArgAttribute::NoUndef, llvm::Attribute::NoUndef), +]; + impl ArgAttributesExt for ArgAttributes { fn apply_attrs_to_llfn(&self, idx: AttributePlace, cx: &CodegenCx<'_, '_>, llfn: &Value) { let mut regular = self.regular; unsafe { + // ABI-affecting attributes must always be applied + for (attr, llattr) in ABI_AFFECTING_ATTRIBUTES { + if regular.contains(attr) { + llattr.apply_llfn(idx, llfn); + } + } + match self.arg_ext { + ArgExtension::None => {} + ArgExtension::Zext => llvm::Attribute::ZExt.apply_llfn(idx, llfn), + ArgExtension::Sext => llvm::Attribute::SExt.apply_llfn(idx, llfn), + } + // Only apply remaining attributes when optimizing + if cx.sess().opts.optimize == config::OptLevel::No { + return; + } let deref = self.pointee_size.bytes(); if deref != 0 { if regular.contains(ArgAttribute::NonNull) { @@ -74,19 +80,14 @@ impl ArgAttributesExt for ArgAttributes { if let Some(align) = self.pointee_align { llvm::LLVMRustAddAlignmentAttr(llfn, idx.as_uint(), align.bytes() as u32); } - regular.for_each_kind(|attr| attr.apply_llfn(idx, llfn)); + for (attr, llattr) in OPTIMIZATION_ATTRIBUTES { + if regular.contains(attr) { + llattr.apply_llfn(idx, llfn); + } + } if regular.contains(ArgAttribute::NoAliasMutRef) && should_use_mutable_noalias(cx) { llvm::Attribute::NoAlias.apply_llfn(idx, llfn); } - match self.arg_ext { - ArgExtension::None => {} - ArgExtension::Zext => { - llvm::Attribute::ZExt.apply_llfn(idx, llfn); - } - ArgExtension::Sext => { - llvm::Attribute::SExt.apply_llfn(idx, llfn); - } - } } } @@ -98,6 +99,21 @@ impl ArgAttributesExt for ArgAttributes { ) { let mut regular = self.regular; unsafe { + // ABI-affecting attributes must always be applied + for (attr, llattr) in ABI_AFFECTING_ATTRIBUTES { + if regular.contains(attr) { + llattr.apply_callsite(idx, callsite); + } + } + match self.arg_ext { + ArgExtension::None => {} + ArgExtension::Zext => llvm::Attribute::ZExt.apply_callsite(idx, callsite), + ArgExtension::Sext => llvm::Attribute::SExt.apply_callsite(idx, callsite), + } + // Only apply remaining attributes when optimizing + if cx.sess().opts.optimize == config::OptLevel::No { + return; + } let deref = self.pointee_size.bytes(); if deref != 0 { if regular.contains(ArgAttribute::NonNull) { @@ -118,19 +134,14 @@ impl ArgAttributesExt for ArgAttributes { align.bytes() as u32, ); } - regular.for_each_kind(|attr| attr.apply_callsite(idx, callsite)); + for (attr, llattr) in OPTIMIZATION_ATTRIBUTES { + if regular.contains(attr) { + llattr.apply_callsite(idx, callsite); + } + } if regular.contains(ArgAttribute::NoAliasMutRef) && should_use_mutable_noalias(cx) { llvm::Attribute::NoAlias.apply_callsite(idx, callsite); } - match self.arg_ext { - ArgExtension::None => {} - ArgExtension::Zext => { - llvm::Attribute::ZExt.apply_callsite(idx, callsite); - } - ArgExtension::Sext => { - llvm::Attribute::SExt.apply_callsite(idx, callsite); - } - } } } } diff --git a/src/test/codegen/function-arguments-noopt.rs b/src/test/codegen/function-arguments-noopt.rs new file mode 100644 index 0000000000000..c8c8888897884 --- /dev/null +++ b/src/test/codegen/function-arguments-noopt.rs @@ -0,0 +1,63 @@ +// compile-flags: -C opt-level=0 -C no-prepopulate-passes + +// This test checks that arguments/returns in opt-level=0 builds, +// while lacking attributes used for optimization, still have ABI-affecting attributes. + +#![crate_type = "lib"] +#![feature(rustc_attrs)] + +pub struct S { + _field: [i32; 8], +} + +// CHECK: define zeroext i1 @boolean(i1 zeroext %x) +#[no_mangle] +pub fn boolean(x: bool) -> bool { + x +} + +// CHECK-LABEL: @boolean_call +#[no_mangle] +pub fn boolean_call(x: bool, f: fn(bool) -> bool) -> bool { +// CHECK: call zeroext i1 %f(i1 zeroext %x) + f(x) +} + +// CHECK: define i32* @borrow(i32* %x) +#[no_mangle] +pub fn borrow(x: &i32) -> &i32 { + x +} + +// CHECK-LABEL: @borrow_call +#[no_mangle] +pub fn borrow_call(x: &i32, f: fn(&i32) -> &i32) -> &i32 { + // CHECK: call i32* %f(i32* %x) + f(x) +} + +// CHECK: define void @struct_(%S* sret(%S){{( %0)?}}, %S* %x) +#[no_mangle] +pub fn struct_(x: S) -> S { + x +} + +// CHECK-LABEL: @struct_call +#[no_mangle] +pub fn struct_call(x: S, f: fn(S) -> S) -> S { + // CHECK: call void %f(%S* sret(%S){{( %0)?}}, %S* %{{.+}}) + f(x) +} + +// CHECK: define { i8, i8 } @enum_(i1 zeroext %x.0, i8 %x.1) +#[no_mangle] +pub fn enum_(x: Option) -> Option { + x +} + +// CHECK-LABEL: @enum_call +#[no_mangle] +pub fn enum_call(x: Option, f: fn(Option) -> Option) -> Option { + // CHECK: call { i8, i8 } %f(i1 zeroext %x.0, i8 %x.1) + f(x) +} From 6e740ae9348508544f41076ea9c7f8b4c3848688 Mon Sep 17 00:00:00 2001 From: Erik Desjardins Date: Sat, 19 Feb 2022 09:59:36 -0500 Subject: [PATCH 2/4] always add align attributes --- compiler/rustc_codegen_llvm/src/abi.rs | 20 ++++++++++---------- src/test/codegen/function-arguments-noopt.rs | 4 ++-- 2 files changed, 12 insertions(+), 12 deletions(-) diff --git a/compiler/rustc_codegen_llvm/src/abi.rs b/compiler/rustc_codegen_llvm/src/abi.rs index a20801914adfa..5b8cc238480e1 100644 --- a/compiler/rustc_codegen_llvm/src/abi.rs +++ b/compiler/rustc_codegen_llvm/src/abi.rs @@ -59,6 +59,9 @@ impl ArgAttributesExt for ArgAttributes { llattr.apply_llfn(idx, llfn); } } + if let Some(align) = self.pointee_align { + llvm::LLVMRustAddAlignmentAttr(llfn, idx.as_uint(), align.bytes() as u32); + } match self.arg_ext { ArgExtension::None => {} ArgExtension::Zext => llvm::Attribute::ZExt.apply_llfn(idx, llfn), @@ -77,9 +80,6 @@ impl ArgAttributesExt for ArgAttributes { } regular -= ArgAttribute::NonNull; } - if let Some(align) = self.pointee_align { - llvm::LLVMRustAddAlignmentAttr(llfn, idx.as_uint(), align.bytes() as u32); - } for (attr, llattr) in OPTIMIZATION_ATTRIBUTES { if regular.contains(attr) { llattr.apply_llfn(idx, llfn); @@ -105,6 +105,13 @@ impl ArgAttributesExt for ArgAttributes { llattr.apply_callsite(idx, callsite); } } + if let Some(align) = self.pointee_align { + llvm::LLVMRustAddAlignmentCallSiteAttr( + callsite, + idx.as_uint(), + align.bytes() as u32, + ); + } match self.arg_ext { ArgExtension::None => {} ArgExtension::Zext => llvm::Attribute::ZExt.apply_callsite(idx, callsite), @@ -127,13 +134,6 @@ impl ArgAttributesExt for ArgAttributes { } regular -= ArgAttribute::NonNull; } - if let Some(align) = self.pointee_align { - llvm::LLVMRustAddAlignmentCallSiteAttr( - callsite, - idx.as_uint(), - align.bytes() as u32, - ); - } for (attr, llattr) in OPTIMIZATION_ATTRIBUTES { if regular.contains(attr) { llattr.apply_callsite(idx, callsite); diff --git a/src/test/codegen/function-arguments-noopt.rs b/src/test/codegen/function-arguments-noopt.rs index c8c8888897884..1d740a4b3290b 100644 --- a/src/test/codegen/function-arguments-noopt.rs +++ b/src/test/codegen/function-arguments-noopt.rs @@ -23,7 +23,7 @@ pub fn boolean_call(x: bool, f: fn(bool) -> bool) -> bool { f(x) } -// CHECK: define i32* @borrow(i32* %x) +// CHECK: define align 4 i32* @borrow(i32* align 4 %x) #[no_mangle] pub fn borrow(x: &i32) -> &i32 { x @@ -32,7 +32,7 @@ pub fn borrow(x: &i32) -> &i32 { // CHECK-LABEL: @borrow_call #[no_mangle] pub fn borrow_call(x: &i32, f: fn(&i32) -> &i32) -> &i32 { - // CHECK: call i32* %f(i32* %x) + // CHECK: call align 4 i32* %f(i32* align 4 %x) f(x) } From b0921f8a0dc50f65ad561ca94da1c09dd5ee293b Mon Sep 17 00:00:00 2001 From: Erik Desjardins Date: Thu, 24 Feb 2022 18:16:10 -0500 Subject: [PATCH 3/4] make tests work on noopt builder --- src/test/codegen/fastcall-inreg.rs | 2 +- src/test/codegen/repr-transparent-aggregates-1.rs | 2 +- src/test/codegen/riscv-abi/riscv64-lp64-lp64f-lp64d-abi.rs | 2 +- src/test/codegen/union-abi.rs | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/test/codegen/fastcall-inreg.rs b/src/test/codegen/fastcall-inreg.rs index ec7a679159286..f550ac11f64ae 100644 --- a/src/test/codegen/fastcall-inreg.rs +++ b/src/test/codegen/fastcall-inreg.rs @@ -2,7 +2,7 @@ // as "inreg" like the C/C++ compilers for the platforms. // x86 only. -// compile-flags: --target i686-unknown-linux-gnu -C no-prepopulate-passes +// compile-flags: --target i686-unknown-linux-gnu -O -C no-prepopulate-passes // needs-llvm-components: x86 #![crate_type = "lib"] diff --git a/src/test/codegen/repr-transparent-aggregates-1.rs b/src/test/codegen/repr-transparent-aggregates-1.rs index a61dad218cd85..4ad3642c03d68 100644 --- a/src/test/codegen/repr-transparent-aggregates-1.rs +++ b/src/test/codegen/repr-transparent-aggregates-1.rs @@ -1,4 +1,4 @@ -// compile-flags: -C no-prepopulate-passes +// compile-flags: -O -C no-prepopulate-passes // // ignore-arm diff --git a/src/test/codegen/riscv-abi/riscv64-lp64-lp64f-lp64d-abi.rs b/src/test/codegen/riscv-abi/riscv64-lp64-lp64f-lp64d-abi.rs index c67406ea69338..faf81b5ae7605 100644 --- a/src/test/codegen/riscv-abi/riscv64-lp64-lp64f-lp64d-abi.rs +++ b/src/test/codegen/riscv-abi/riscv64-lp64-lp64f-lp64d-abi.rs @@ -1,4 +1,4 @@ -// compile-flags: --target riscv64gc-unknown-linux-gnu -C no-prepopulate-passes +// compile-flags: --target riscv64gc-unknown-linux-gnu -O -C no-prepopulate-passes // needs-llvm-components: riscv #![crate_type = "lib"] diff --git a/src/test/codegen/union-abi.rs b/src/test/codegen/union-abi.rs index bb87d263bdf3d..99576a5f57e90 100644 --- a/src/test/codegen/union-abi.rs +++ b/src/test/codegen/union-abi.rs @@ -1,5 +1,5 @@ // ignore-emscripten vectors passed directly -// compile-flags: -C no-prepopulate-passes +// compile-flags: -O -C no-prepopulate-passes // This test that using union forward the abi of the inner type, as // discussed in #54668 From 945276c92076ea82747b4c8d0b19206c2940e7d8 Mon Sep 17 00:00:00 2001 From: Erik Desjardins Date: Fri, 25 Feb 2022 19:24:59 -0500 Subject: [PATCH 4/4] avoid test failure on targets where all functions are dso_local (e.g. wasm) --- src/test/codegen/function-arguments-noopt.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/test/codegen/function-arguments-noopt.rs b/src/test/codegen/function-arguments-noopt.rs index 1d740a4b3290b..94561288dc5b9 100644 --- a/src/test/codegen/function-arguments-noopt.rs +++ b/src/test/codegen/function-arguments-noopt.rs @@ -10,7 +10,7 @@ pub struct S { _field: [i32; 8], } -// CHECK: define zeroext i1 @boolean(i1 zeroext %x) +// CHECK: zeroext i1 @boolean(i1 zeroext %x) #[no_mangle] pub fn boolean(x: bool) -> bool { x @@ -23,7 +23,7 @@ pub fn boolean_call(x: bool, f: fn(bool) -> bool) -> bool { f(x) } -// CHECK: define align 4 i32* @borrow(i32* align 4 %x) +// CHECK: align 4 i32* @borrow(i32* align 4 %x) #[no_mangle] pub fn borrow(x: &i32) -> &i32 { x @@ -36,7 +36,7 @@ pub fn borrow_call(x: &i32, f: fn(&i32) -> &i32) -> &i32 { f(x) } -// CHECK: define void @struct_(%S* sret(%S){{( %0)?}}, %S* %x) +// CHECK: void @struct_(%S* sret(%S){{( %0)?}}, %S* %x) #[no_mangle] pub fn struct_(x: S) -> S { x @@ -49,7 +49,7 @@ pub fn struct_call(x: S, f: fn(S) -> S) -> S { f(x) } -// CHECK: define { i8, i8 } @enum_(i1 zeroext %x.0, i8 %x.1) +// CHECK: { i8, i8 } @enum_(i1 zeroext %x.0, i8 %x.1) #[no_mangle] pub fn enum_(x: Option) -> Option { x