Skip to content

Commit 9c88898

Browse files
authored
Auto merge of #34899 - michaelwoerister:always_internalize_symbols, r=eddyb
Run base::internalize_symbols() even for single-codegen-unit crates. The initial linkage-assignment (especially for closures) is a conservative one that makes some symbols more visible than they need to be. While this is not a correctness problem, it does force the LLVM inliner to be more conservative too, which results in poor performance. Once translation is based solely on MIR, it will be easier to also make the initial linkage assignment a better fitting one. Until then `internalize_symbols()` does a good job of preventing most performance regressions. This should solve the regressions reported in #34891 and maybe also those in #34831. As a side-effect, this will also solve most of the problematic cases described in #34793. Not reliably so, however. For that, we still need a solution like the one implement in #34830. cc @rust-lang/compiler
2 parents 06ca016 + 22f77a9 commit 9c88898

File tree

1 file changed

+15
-21
lines changed

1 file changed

+15
-21
lines changed

Diff for: src/librustc_trans/base.rs

+15-21
Original file line numberDiff line numberDiff line change
@@ -2270,12 +2270,9 @@ fn internalize_symbols(cx: &CrateContextList, reachable: &HashSet<&str>) {
22702270
let is_decl = llvm::LLVMIsDeclaration(val) != 0;
22712271

22722272
if is_decl || is_available_externally {
2273-
let name = CStr::from_ptr(llvm::LLVMGetValueName(val))
2274-
.to_bytes()
2275-
.to_vec();
2273+
let name = CStr::from_ptr(llvm::LLVMGetValueName(val));
22762274
declared.insert(name);
22772275
}
2278-
22792276
}
22802277
}
22812278

@@ -2286,21 +2283,21 @@ fn internalize_symbols(cx: &CrateContextList, reachable: &HashSet<&str>) {
22862283
for val in iter_globals(ccx.llmod()).chain(iter_functions(ccx.llmod())) {
22872284
let linkage = llvm::LLVMGetLinkage(val);
22882285

2289-
let is_external = linkage == llvm::ExternalLinkage as c_uint;
2290-
let is_weak_odr = linkage == llvm::WeakODRLinkage as c_uint;
2291-
let is_decl = llvm::LLVMIsDeclaration(val) != 0;
2292-
2293-
// We only care about external definitions.
2294-
if (is_external || is_weak_odr) && !is_decl {
2286+
let is_externally_visible = (linkage == llvm::ExternalLinkage as c_uint) ||
2287+
(linkage == llvm::LinkOnceODRLinkage as c_uint) ||
2288+
(linkage == llvm::WeakODRLinkage as c_uint);
2289+
let is_definition = llvm::LLVMIsDeclaration(val) != 0;
22952290

2296-
let name = CStr::from_ptr(llvm::LLVMGetValueName(val))
2297-
.to_bytes()
2298-
.to_vec();
2291+
// If this is a definition (as opposed to just a declaration)
2292+
// and externally visible, check if we can internalize it
2293+
if is_definition && is_externally_visible {
2294+
let name_cstr = CStr::from_ptr(llvm::LLVMGetValueName(val));
2295+
let name_str = name_cstr.to_str().unwrap();
22992296

2300-
let is_declared = declared.contains(&name);
2301-
let reachable = reachable.contains(str::from_utf8(&name).unwrap());
2297+
let is_referenced_somewhere = declared.contains(&name_cstr);
2298+
let is_reachable = reachable.contains(name_str);
23022299

2303-
if !is_declared && !reachable {
2300+
if !is_referenced_somewhere && !is_reachable {
23042301
llvm::SetLinkage(val, llvm::InternalLinkage);
23052302
llvm::SetDLLStorageClass(val, llvm::DefaultStorageClass);
23062303
llvm::UnsetComdat(val);
@@ -2488,7 +2485,6 @@ pub fn trans_crate<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>,
24882485
// Run the translation item collector and partition the collected items into
24892486
// codegen units.
24902487
let (codegen_units, symbol_map) = collect_and_partition_translation_items(&shared_ccx);
2491-
let codegen_unit_count = codegen_units.len();
24922488

24932489
let symbol_map = Rc::new(symbol_map);
24942490

@@ -2620,10 +2616,8 @@ pub fn trans_crate<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>,
26202616
}));
26212617
}
26222618

2623-
if codegen_unit_count > 1 {
2624-
internalize_symbols(&crate_context_list,
2625-
&reachable_symbols.iter().map(|x| &x[..]).collect());
2626-
}
2619+
internalize_symbols(&crate_context_list,
2620+
&reachable_symbols.iter().map(|x| &x[..]).collect());
26272621

26282622
if sess.target.target.options.is_like_msvc &&
26292623
sess.crate_types.borrow().iter().any(|ct| *ct == config::CrateTypeRlib) {

0 commit comments

Comments
 (0)