Rollup merge of #137313 - oli-obk:push-ywvuqkxuqyom, r=petrochenkov

Some codegen_llvm cleanups

Using some more safe wrappers and thus being able to remove a large unsafe block.

As a next step we should probably look into safe extern fns

Author: matthiaskrgr

Diff for: compiler/rustc_codegen_llvm/src/back/lto.rs

@@ -362,8 +362,8 @@ fn fat_lto(
                 ptr as *const *const libc::c_char,
                 symbols_below_threshold.len() as libc::size_t,
             );
-            save_temp_bitcode(cgcx, &module, "lto.after-restriction");
         }
+        save_temp_bitcode(cgcx, &module, "lto.after-restriction");
     }
 
     Ok(LtoModuleCodegen::Fat(module))

Diff for: compiler/rustc_codegen_llvm/src/callee.rs

@@ -66,9 +66,7 @@ pub(crate) fn get_fn<'ll, 'tcx>(cx: &CodegenCx<'ll, 'tcx>, instance: Instance<'t
             // LLVM will prefix the name with `__imp_`. Ideally, we'd like the
             // existing logic below to set the Storage Class, but it has an
             // exemption for MinGW for backwards compatibility.
-            unsafe {
-                llvm::LLVMSetDLLStorageClass(llfn, llvm::DLLStorageClass::DllImport);
-            }
+            llvm::set_dllimport_storage_class(llfn);
             llfn
         } else {
             cx.declare_fn(sym, fn_abi, Some(instance))
@@ -99,65 +97,61 @@ pub(crate) fn get_fn<'ll, 'tcx>(cx: &CodegenCx<'ll, 'tcx>, instance: Instance<'t
         // has been applied to the definition (wherever that definition may be).
 
         llvm::set_linkage(llfn, llvm::Linkage::ExternalLinkage);
-        unsafe {
-            let is_generic = instance.args.non_erasable_generics().next().is_some();
-
-            let is_hidden = if is_generic {
-                // This is a monomorphization of a generic function.
-                if !(cx.tcx.sess.opts.share_generics()
-                    || tcx.codegen_fn_attrs(instance_def_id).inline
-                        == rustc_attr_parsing::InlineAttr::Never)
-                {
-                    // When not sharing generics, all instances are in the same
-                    // crate and have hidden visibility.
-                    true
-                } else {
-                    if let Some(instance_def_id) = instance_def_id.as_local() {
-                        // This is a monomorphization of a generic function
-                        // defined in the current crate. It is hidden if:
-                        // - the definition is unreachable for downstream
-                        //   crates, or
-                        // - the current crate does not re-export generics
-                        //   (because the crate is a C library or executable)
-                        cx.tcx.is_unreachable_local_definition(instance_def_id)
-                            || !cx.tcx.local_crate_exports_generics()
-                    } else {
-                        // This is a monomorphization of a generic function
-                        // defined in an upstream crate. It is hidden if:
-                        // - it is instantiated in this crate, and
-                        // - the current crate does not re-export generics
-                        instance.upstream_monomorphization(tcx).is_none()
-                            && !cx.tcx.local_crate_exports_generics()
-                    }
-                }
-            } else {
-                // This is a non-generic function. It is hidden if:
-                // - it is instantiated in the local crate, and
-                //   - it is defined an upstream crate (non-local), or
-                //   - it is not reachable
-                cx.tcx.is_codegened_item(instance_def_id)
-                    && (!instance_def_id.is_local()
-                        || !cx.tcx.is_reachable_non_generic(instance_def_id))
-            };
-            if is_hidden {
-                llvm::set_visibility(llfn, llvm::Visibility::Hidden);
-            }
+        let is_generic = instance.args.non_erasable_generics().next().is_some();
 
-            // MinGW: For backward compatibility we rely on the linker to decide whether it
-            // should use dllimport for functions.
-            if cx.use_dll_storage_attrs
-                && let Some(library) = tcx.native_library(instance_def_id)
-                && library.kind.is_dllimport()
-                && !matches!(tcx.sess.target.env.as_ref(), "gnu" | "uclibc")
+        let is_hidden = if is_generic {
+            // This is a monomorphization of a generic function.
+            if !(cx.tcx.sess.opts.share_generics()
+                || tcx.codegen_fn_attrs(instance_def_id).inline
+                    == rustc_attr_parsing::InlineAttr::Never)
             {
-                llvm::LLVMSetDLLStorageClass(llfn, llvm::DLLStorageClass::DllImport);
+                // When not sharing generics, all instances are in the same
+                // crate and have hidden visibility.
+                true
+            } else {
+                if let Some(instance_def_id) = instance_def_id.as_local() {
+                    // This is a monomorphization of a generic function
+                    // defined in the current crate. It is hidden if:
+                    // - the definition is unreachable for downstream
+                    //   crates, or
+                    // - the current crate does not re-export generics
+                    //   (because the crate is a C library or executable)
+                    cx.tcx.is_unreachable_local_definition(instance_def_id)
+                        || !cx.tcx.local_crate_exports_generics()
+                } else {
+                    // This is a monomorphization of a generic function
+                    // defined in an upstream crate. It is hidden if:
+                    // - it is instantiated in this crate, and
+                    // - the current crate does not re-export generics
+                    instance.upstream_monomorphization(tcx).is_none()
+                        && !cx.tcx.local_crate_exports_generics()
+                }
             }
+        } else {
+            // This is a non-generic function. It is hidden if:
+            // - it is instantiated in the local crate, and
+            //   - it is defined an upstream crate (non-local), or
+            //   - it is not reachable
+            cx.tcx.is_codegened_item(instance_def_id)
+                && (!instance_def_id.is_local()
+                    || !cx.tcx.is_reachable_non_generic(instance_def_id))
+        };
+        if is_hidden {
+            llvm::set_visibility(llfn, llvm::Visibility::Hidden);
+        }
 
-            if cx.should_assume_dso_local(llfn, true) {
-                llvm::LLVMRustSetDSOLocal(llfn, true);
-            }
+        // MinGW: For backward compatibility we rely on the linker to decide whether it
+        // should use dllimport for functions.
+        if cx.use_dll_storage_attrs
+            && let Some(library) = tcx.native_library(instance_def_id)
+            && library.kind.is_dllimport()
+            && !matches!(tcx.sess.target.env.as_ref(), "gnu" | "uclibc")
+        {
+            llvm::set_dllimport_storage_class(llfn);
         }
 
+        cx.assume_dso_local(llfn, true);
+
         llfn
     };
 

Diff for: compiler/rustc_codegen_llvm/src/consts.rs

@@ -336,12 +336,7 @@ impl<'ll> CodegenCx<'ll, '_> {
             llvm::set_thread_local_mode(g, self.tls_model);
         }
 
-        let dso_local = self.should_assume_dso_local(g, true);
-        if dso_local {
-            unsafe {
-                llvm::LLVMRustSetDSOLocal(g, true);
-            }
-        }
+        let dso_local = self.assume_dso_local(g, true);
 
         if !def_id.is_local() {
             let needs_dll_storage_attr = self.use_dll_storage_attrs
@@ -375,9 +370,7 @@ impl<'ll> CodegenCx<'ll, '_> {
                 // is actually present in the current crate. We can find out via the
                 // is_codegened_item query.
                 if !self.tcx.is_codegened_item(def_id) {
-                    unsafe {
-                        llvm::LLVMSetDLLStorageClass(g, llvm::DLLStorageClass::DllImport);
-                    }
+                    llvm::set_dllimport_storage_class(g);
                 }
             }
         }
@@ -387,9 +380,7 @@ impl<'ll> CodegenCx<'ll, '_> {
             && library.kind.is_dllimport()
         {
             // For foreign (native) libs we know the exact storage type to use.
-            unsafe {
-                llvm::LLVMSetDLLStorageClass(g, llvm::DLLStorageClass::DllImport);
-            }
+            llvm::set_dllimport_storage_class(g);
         }
 
         self.instances.borrow_mut().insert(instance, g);
@@ -460,9 +451,7 @@ impl<'ll> CodegenCx<'ll, '_> {
             set_global_alignment(self, g, alloc.align);
             llvm::set_initializer(g, v);
 
-            if self.should_assume_dso_local(g, true) {
-                llvm::LLVMRustSetDSOLocal(g, true);
-            }
+            self.assume_dso_local(g, true);
 
             // Forward the allocation's mutability (picked by the const interner) to LLVM.
             if alloc.mutability.is_not() {

Diff for: compiler/rustc_codegen_llvm/src/llvm/mod.rs

@@ -403,3 +403,15 @@ pub(crate) fn add_module_flag_str(
         );
     }
 }
+
+pub(crate) fn set_dllimport_storage_class<'ll>(v: &'ll Value) {
+    unsafe {
+        LLVMSetDLLStorageClass(v, DLLStorageClass::DllImport);
+    }
+}
+
+pub(crate) fn set_dso_local<'ll>(v: &'ll Value) {
+    unsafe {
+        LLVMRustSetDSOLocal(v, true);
+    }
+}

Diff for: compiler/rustc_codegen_llvm/src/mono_item.rs

@@ -38,11 +38,7 @@ impl<'tcx> PreDefineCodegenMethods<'tcx> for CodegenCx<'_, 'tcx> {
 
         llvm::set_linkage(g, base::linkage_to_llvm(linkage));
         llvm::set_visibility(g, base::visibility_to_llvm(visibility));
-        unsafe {
-            if self.should_assume_dso_local(g, false) {
-                llvm::LLVMRustSetDSOLocal(g, true);
-            }
-        }
+        self.assume_dso_local(g, false);
 
         self.instances.borrow_mut().insert(instance, g);
     }
@@ -79,9 +75,7 @@ impl<'tcx> PreDefineCodegenMethods<'tcx> for CodegenCx<'_, 'tcx> {
 
         debug!("predefine_fn: instance = {:?}", instance);
 
-        if self.should_assume_dso_local(lldecl, false) {
-            unsafe { llvm::LLVMRustSetDSOLocal(lldecl, true) };
-        }
+        self.assume_dso_local(lldecl, false);
 
         self.instances.borrow_mut().insert(instance, lldecl);
     }
@@ -90,11 +84,16 @@ impl<'tcx> PreDefineCodegenMethods<'tcx> for CodegenCx<'_, 'tcx> {
 impl CodegenCx<'_, '_> {
     /// Whether a definition or declaration can be assumed to be local to a group of
     /// libraries that form a single DSO or executable.
-    pub(crate) fn should_assume_dso_local(
-        &self,
-        llval: &llvm::Value,
-        is_declaration: bool,
-    ) -> bool {
+    /// Marks the local as DSO if so.
+    pub(crate) fn assume_dso_local(&self, llval: &llvm::Value, is_declaration: bool) -> bool {
+        let assume = self.should_assume_dso_local(llval, is_declaration);
+        if assume {
+            llvm::set_dso_local(llval);
+        }
+        assume
+    }
+
+    fn should_assume_dso_local(&self, llval: &llvm::Value, is_declaration: bool) -> bool {
         let linkage = llvm::get_linkage(llval);
         let visibility = llvm::get_visibility(llval);