Skip to content

Commit

Permalink
Rollup merge of rust-lang#137313 - oli-obk:push-ywvuqkxuqyom, r=petro…
Browse files Browse the repository at this point in the history 
…chenkov

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
  • Loading branch information
@workingjubilee
workingjubilee authored Feb 20, 2025
2 parents faeb221 + ce7f58b commit 4a32c28
Show file tree
Hide file tree
Showing 5 changed files with 79 additions and 85 deletions.
2 changes: 1 addition & 1 deletion compiler/rustc_codegen_llvm/src/back/lto.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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))
Expand Down
106 changes: 50 additions & 56 deletions compiler/rustc_codegen_llvm/src/callee.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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))
Expand Down Expand Up @@ -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
};

Expand Down
19 changes: 4 additions & 15 deletions compiler/rustc_codegen_llvm/src/consts.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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);
}
}
}
Expand All @@ -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);
Expand Down Expand Up @@ -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() {
Expand Down
12 changes: 12 additions & 0 deletions compiler/rustc_codegen_llvm/src/llvm/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}
25 changes: 12 additions & 13 deletions compiler/rustc_codegen_llvm/src/mono_item.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand Down Expand Up @@ -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);
}
Expand All @@ -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);

Expand Down

0 comments on commit 4a32c28

Please sign in to comment.