Auto merge of rust-lang#94229 - erikdesjardins:rem2, r=nikic

Remove LLVM attribute removal

This was necessary before, because `declare_raw_fn` would always apply
the default optimization attributes to every declared function.
Then `attributes::from_fn_attrs` would have to remove the default
attributes in the case of, e.g. `#[optimize(speed)]` in a `-Os` build.
(see [`src/test/codegen/optimize-attr-1.rs`](https://github.com/rust-lang/rust/blob/03a8cc7df1d65554a4d40825b0490c93ac0f0236/src/test/codegen/optimize-attr-1.rs#L33))

However, every relevant callsite of `declare_raw_fn` (i.e. where we
actually generate code for the function, and not e.g. a call to an
intrinsic, where optimization attributes don't [?] matter)
calls `from_fn_attrs`, so we can remove the attribute setting
from `declare_raw_fn`, and rely on `from_fn_attrs` to apply the correct
attributes all at once.

r? `@ghost` (blocked on rust-lang#94221)
`@rustbot` label S-blocked

Author: bors

compiler/rustc_codegen_llvm/src/attributes.rs

@@ -28,12 +28,6 @@ pub fn apply_to_llfn(llfn: &Value, idx: AttributePlace, attrs: &[&Attribute]) {
     }
 }
 
-pub fn remove_from_llfn(llfn: &Value, idx: AttributePlace, attrs: &[AttributeKind]) {
-    if !attrs.is_empty() {
-        llvm::RemoveFunctionAttributes(llfn, idx, attrs);
-    }
-}
-
 pub fn apply_to_callsite(callsite: &Value, idx: AttributePlace, attrs: &[&Attribute]) {
     if !attrs.is_empty() {
         llvm::AddCallSiteAttributes(callsite, idx, attrs);
@@ -215,38 +209,23 @@ pub fn non_lazy_bind_attr<'ll>(cx: &CodegenCx<'ll, '_>) -> Option<&'ll Attribute
     }
 }
 
-/// Returns attributes to remove and to add, respectively,
-/// to set the default optimizations attrs on a function.
+/// Get the default optimizations attrs for a function.
 #[inline]
 pub(crate) fn default_optimisation_attrs<'ll>(
     cx: &CodegenCx<'ll, '_>,
-) -> (
-    // Attributes to remove
-    SmallVec<[AttributeKind; 3]>,
-    // Attributes to add
-    SmallVec<[&'ll Attribute; 2]>,
-) {
-    let mut to_remove = SmallVec::new();
-    let mut to_add = SmallVec::new();
+) -> SmallVec<[&'ll Attribute; 2]> {
+    let mut attrs = SmallVec::new();
     match cx.sess().opts.optimize {
         OptLevel::Size => {
-            to_remove.push(llvm::AttributeKind::MinSize);
-            to_add.push(llvm::AttributeKind::OptimizeForSize.create_attr(cx.llcx));
-            to_remove.push(llvm::AttributeKind::OptimizeNone);
+            attrs.push(llvm::AttributeKind::OptimizeForSize.create_attr(cx.llcx));
         }
         OptLevel::SizeMin => {
-            to_add.push(llvm::AttributeKind::MinSize.create_attr(cx.llcx));
-            to_add.push(llvm::AttributeKind::OptimizeForSize.create_attr(cx.llcx));
-            to_remove.push(llvm::AttributeKind::OptimizeNone);
-        }
-        OptLevel::No => {
-            to_remove.push(llvm::AttributeKind::MinSize);
-            to_remove.push(llvm::AttributeKind::OptimizeForSize);
-            to_remove.push(llvm::AttributeKind::OptimizeNone);
+            attrs.push(llvm::AttributeKind::MinSize.create_attr(cx.llcx));
+            attrs.push(llvm::AttributeKind::OptimizeForSize.create_attr(cx.llcx));
         }
         _ => {}
     }
-    (to_remove, to_add)
+    attrs
 }
 
 /// Composite function which sets LLVM attributes for function depending on its AST (`#[attribute]`)
@@ -258,25 +237,17 @@ pub fn from_fn_attrs<'ll, 'tcx>(
 ) {
     let codegen_fn_attrs = cx.tcx.codegen_fn_attrs(instance.def_id());
 
-    let mut to_remove = SmallVec::<[_; 4]>::new();
     let mut to_add = SmallVec::<[_; 16]>::new();
 
     match codegen_fn_attrs.optimize {
         OptimizeAttr::None => {
-            let (to_remove_opt, to_add_opt) = default_optimisation_attrs(cx);
-            to_remove.extend(to_remove_opt);
-            to_add.extend(to_add_opt);
-        }
-        OptimizeAttr::Speed => {
-            to_remove.push(llvm::AttributeKind::MinSize);
-            to_remove.push(llvm::AttributeKind::OptimizeForSize);
-            to_remove.push(llvm::AttributeKind::OptimizeNone);
+            to_add.extend(default_optimisation_attrs(cx));
         }
         OptimizeAttr::Size => {
             to_add.push(llvm::AttributeKind::MinSize.create_attr(cx.llcx));
             to_add.push(llvm::AttributeKind::OptimizeForSize.create_attr(cx.llcx));
-            to_remove.push(llvm::AttributeKind::OptimizeNone);
         }
+        OptimizeAttr::Speed => {}
     }
 
     let inline = if codegen_fn_attrs.flags.contains(CodegenFnAttrFlags::NAKED) {
@@ -421,7 +392,6 @@ pub fn from_fn_attrs<'ll, 'tcx>(
         to_add.push(llvm::CreateAttrStringValue(cx.llcx, cstr!("target-features"), &val));
     }
 
-    attributes::remove_from_llfn(llfn, Function, &to_remove);
     attributes::apply_to_llfn(llfn, Function, &to_add);
 }
 

compiler/rustc_codegen_llvm/src/declare.rs

@@ -41,21 +41,15 @@ fn declare_raw_fn<'ll>(
     llvm::SetFunctionCallConv(llfn, callconv);
     llvm::SetUnnamedAddress(llfn, unnamed);
 
-    let mut attrs_to_remove = SmallVec::<[_; 4]>::new();
-    let mut attrs_to_add = SmallVec::<[_; 4]>::new();
+    let mut attrs = SmallVec::<[_; 4]>::new();
 
     if cx.tcx.sess.opts.cg.no_redzone.unwrap_or(cx.tcx.sess.target.disable_redzone) {
-        attrs_to_add.push(llvm::AttributeKind::NoRedZone.create_attr(cx.llcx));
+        attrs.push(llvm::AttributeKind::NoRedZone.create_attr(cx.llcx));
     }
 
-    let (to_remove, to_add) = attributes::default_optimisation_attrs(cx);
-    attrs_to_remove.extend(to_remove);
-    attrs_to_add.extend(to_add);
+    attrs.extend(attributes::non_lazy_bind_attr(cx));
 
-    attrs_to_add.extend(attributes::non_lazy_bind_attr(cx));
-
-    attributes::remove_from_llfn(llfn, Function, &attrs_to_remove);
-    attributes::apply_to_llfn(llfn, Function, &attrs_to_add);
+    attributes::apply_to_llfn(llfn, Function, &attrs);
 
     llfn
 }

compiler/rustc_codegen_llvm/src/llvm/ffi.rs

@@ -1202,12 +1202,6 @@ extern "C" {
         Attrs: *const &'a Attribute,
         AttrsLen: size_t,
     );
-    pub fn LLVMRustRemoveFunctionAttributes(
-        Fn: &Value,
-        index: c_uint,
-        Attrs: *const AttributeKind,
-        AttrsLen: size_t,
-    );
 
     // Operations on parameters
     pub fn LLVMIsAArgument(Val: &Value) -> Option<&Value>;

compiler/rustc_codegen_llvm/src/llvm/mod.rs

@@ -37,12 +37,6 @@ pub fn AddFunctionAttributes<'ll>(llfn: &'ll Value, idx: AttributePlace, attrs:
     }
 }
 
-pub fn RemoveFunctionAttributes(llfn: &Value, idx: AttributePlace, attrs: &[AttributeKind]) {
-    unsafe {
-        LLVMRustRemoveFunctionAttributes(llfn, idx.as_uint(), attrs.as_ptr(), attrs.len());
-    }
-}
-
 pub fn AddCallSiteAttributes<'ll>(
     callsite: &'ll Value,
     idx: AttributePlace,

compiler/rustc_llvm/llvm-wrapper/RustWrapper.cpp

@@ -250,38 +250,12 @@ template<typename T> static inline void AddAttributes(T *t, unsigned Index,
   t->setAttributes(PALNew);
 }
 
-template<typename T> static inline void RemoveAttributes(T *t, unsigned Index,
-                                                         LLVMRustAttribute *RustAttrs,
-                                                         size_t RustAttrsLen) {
-  AttributeList PAL = t->getAttributes();
-  AttributeList PALNew;
-#if LLVM_VERSION_LT(14, 0)
-  AttrBuilder B;
-  for (LLVMRustAttribute RustAttr : makeArrayRef(RustAttrs, RustAttrsLen))
-    B.addAttribute(fromRust(RustAttr));
-  PALNew = PAL.removeAttributes(t->getContext(), Index, B);
-#else
-  AttributeMask Mask;
-  for (LLVMRustAttribute RustAttr : makeArrayRef(RustAttrs, RustAttrsLen))
-    Mask.addAttribute(fromRust(RustAttr));
-  PALNew = PAL.removeAttributesAtIndex(t->getContext(), Index, Mask);
-#endif
-  t->setAttributes(PALNew);
-}
-
 extern "C" void LLVMRustAddFunctionAttributes(LLVMValueRef Fn, unsigned Index,
                                               LLVMAttributeRef *Attrs, size_t AttrsLen) {
   Function *F = unwrap<Function>(Fn);
   AddAttributes(F, Index, Attrs, AttrsLen);
 }
 
-extern "C" void LLVMRustRemoveFunctionAttributes(LLVMValueRef Fn, unsigned Index,
-                                                 LLVMRustAttribute *RustAttrs,
-                                                 size_t RustAttrsLen) {
-  Function *F = unwrap<Function>(Fn);
-  RemoveAttributes(F, Index, RustAttrs, RustAttrsLen);
-}
-
 extern "C" void LLVMRustAddCallSiteAttributes(LLVMValueRef Instr, unsigned Index,
                                               LLVMAttributeRef *Attrs, size_t AttrsLen) {
   CallBase *Call = unwrap<CallBase>(Instr);