Auto merge of #94127 - erikdesjardins:debugattr, r=nikic

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 perf regression from adding noundef (#93670 (comment), #94106). r? `@nikic`
rust-lang · Feb 26, 2022 · 8128e91 · 8128e91
2 parents d5a9bc9 + 945276c
commit 8128e91
Show file tree

Hide file tree

Showing 6 changed files with 127 additions and 53 deletions.
diff --git a/compiler/rustc_codegen_llvm/src/abi.rs b/compiler/rustc_codegen_llvm/src/abi.rs
@@ -13,34 +13,14 @@ 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};
 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<F>(&self, f: F)
-    where
-        F: FnMut(llvm::Attribute);
-}
-
-impl ArgAttributeExt for ArgAttribute {
-    fn for_each_kind<F>(&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,39 @@ 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);
+                }
+            }
+            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),
+                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) {
@@ -71,22 +80,14 @@ 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);
+                }
             }
-            regular.for_each_kind(|attr| attr.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,28 @@ 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);
+                }
+            }
+            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),
+                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) {
@@ -111,26 +134,14 @@ 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);
+                }
             }
-            regular.for_each_kind(|attr| attr.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/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/function-arguments-noopt.rs 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: 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: align 4 i32* @borrow(i32* align 4 %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 align 4 i32* %f(i32* align 4 %x)
+  f(x)
+}
+
+// CHECK: 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: { i8, i8 } @enum_(i1 zeroext %x.0, i8 %x.1)
+#[no_mangle]
+pub fn enum_(x: Option<u8>) -> Option<u8> {
+  x
+}
+
+// CHECK-LABEL: @enum_call
+#[no_mangle]
+pub fn enum_call(x: Option<u8>, f: fn(Option<u8>) -> Option<u8>) -> Option<u8> {
+  // CHECK: call { i8, i8 } %f(i1 zeroext %x.0, i8 %x.1)
+  f(x)
+}
diff --git 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
@@ -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
@@ -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