Skip to content

Commit

Permalink
Auto merge of #118568 - DianQK:no-builtins-symbols, r=<try>
Browse files Browse the repository at this point in the history
Avoid adding builtin functions to `symbols.o`

We found performance regressions in #113923. The problem seems to be that `--gc-sections` does not remove these symbols. I tested that lld removes these symbols, but ld and gold do not.

I found that `used` adds symbols to `symbols.o` at https://github.com/rust-lang/rust/blob/3e202ead604be31f4c1a5798a296953d3159da7e/compiler/rustc_codegen_ssa/src/back/linker.rs#L1786-L1791.
The PR removes builtin functions.

Note that under LTO, ld still preserves these symbols. (lld will still remove them.)

The first commit also fixes #118559. But I think the second commit also makes sense.
  • Loading branch information
bors committed Dec 5, 2023
2 parents da1da3f + ca0738f commit 3a9fb17
Show file tree
Hide file tree
Showing 6 changed files with 30 additions and 4 deletions.
2 changes: 1 addition & 1 deletion compiler/rustc_codegen_llvm/src/back/lto.rs
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ fn prepare_lto(
};

let symbol_filter = &|&(ref name, info): &(String, SymbolExportInfo)| {
if info.level.is_below_threshold(export_threshold) || info.used {
if info.level.is_below_threshold(export_threshold) || info.used || info.used_compiler {
Some(CString::new(name.as_str()).unwrap())
} else {
None
Expand Down
18 changes: 15 additions & 3 deletions compiler/rustc_codegen_ssa/src/back/symbol_export.rs
Original file line number Diff line number Diff line change
Expand Up @@ -105,20 +105,21 @@ fn reachable_non_generics_provider(tcx: TyCtxt<'_>, _: LocalCrate) -> DefIdMap<S
}
})
.map(|def_id| {
let codegen_attrs = tcx.codegen_fn_attrs(def_id.to_def_id());
// We won't link right if this symbol is stripped during LTO.
let name = tcx.symbol_name(Instance::mono(tcx, def_id.to_def_id())).name;
// We have to preserve the symbols of the built-in functions during LTO.
let is_builtin_fn = is_compiler_builtins
&& symbol_export_level(tcx, def_id.to_def_id())
.is_below_threshold(SymbolExportLevel::C);
let used = is_builtin_fn || name == "rust_eh_personality";
.is_below_threshold(SymbolExportLevel::C)
&& codegen_attrs.flags.contains(CodegenFnAttrFlags::NO_MANGLE);
let used = name == "rust_eh_personality";

let export_level = if special_runtime_crate {
SymbolExportLevel::Rust
} else {
symbol_export_level(tcx, def_id.to_def_id())
};
let codegen_attrs = tcx.codegen_fn_attrs(def_id.to_def_id());
debug!(
"EXPORTED SYMBOL (local): {} ({:?})",
tcx.symbol_name(Instance::mono(tcx, def_id.to_def_id())),
Expand All @@ -138,6 +139,7 @@ fn reachable_non_generics_provider(tcx: TyCtxt<'_>, _: LocalCrate) -> DefIdMap<S
used: codegen_attrs.flags.contains(CodegenFnAttrFlags::USED)
|| codegen_attrs.flags.contains(CodegenFnAttrFlags::USED_LINKER)
|| used,
used_compiler: is_builtin_fn,
};
(def_id.to_def_id(), info)
})
Expand All @@ -150,6 +152,7 @@ fn reachable_non_generics_provider(tcx: TyCtxt<'_>, _: LocalCrate) -> DefIdMap<S
level: SymbolExportLevel::C,
kind: SymbolExportKind::Data,
used: false,
used_compiler: false,
},
);
}
Expand Down Expand Up @@ -198,6 +201,7 @@ fn exported_symbols_provider_local(
level: info.level,
kind: SymbolExportKind::Text,
used: info.used,
used_compiler: false,
},
)
})
Expand All @@ -214,6 +218,7 @@ fn exported_symbols_provider_local(
level: SymbolExportLevel::C,
kind: SymbolExportKind::Text,
used: false,
used_compiler: false,
},
));
}
Expand All @@ -233,6 +238,7 @@ fn exported_symbols_provider_local(
level: SymbolExportLevel::Rust,
kind: SymbolExportKind::Text,
used: false,
used_compiler: false,
},
));
}
Expand All @@ -245,6 +251,7 @@ fn exported_symbols_provider_local(
level: SymbolExportLevel::Rust,
kind: SymbolExportKind::Data,
used: false,
used_compiler: false,
},
))
}
Expand All @@ -264,6 +271,7 @@ fn exported_symbols_provider_local(
level: SymbolExportLevel::C,
kind: SymbolExportKind::Data,
used: false,
used_compiler: false,
},
)
}));
Expand All @@ -289,6 +297,7 @@ fn exported_symbols_provider_local(
level: SymbolExportLevel::C,
kind: SymbolExportKind::Data,
used: false,
used_compiler: false,
},
)
}));
Expand All @@ -306,6 +315,7 @@ fn exported_symbols_provider_local(
level: SymbolExportLevel::C,
kind: SymbolExportKind::Data,
used: true,
used_compiler: false,
},
));
}
Expand Down Expand Up @@ -346,6 +356,7 @@ fn exported_symbols_provider_local(
level: SymbolExportLevel::Rust,
kind: SymbolExportKind::Text,
used: false,
used_compiler: false,
},
));
}
Expand All @@ -362,6 +373,7 @@ fn exported_symbols_provider_local(
level: SymbolExportLevel::Rust,
kind: SymbolExportKind::Text,
used: false,
used_compiler: false,
},
));
}
Expand Down
5 changes: 5 additions & 0 deletions compiler/rustc_middle/src/middle/exported_symbols.rs
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,12 @@ pub enum SymbolExportKind {
pub struct SymbolExportInfo {
pub level: SymbolExportLevel,
pub kind: SymbolExportKind,
/// Used to mark these symbols not to be internalized by LTO. These symbols
/// are also added to `symbols.o` to avoid circular dependencies when linking.
pub used: bool,
/// Also used to mark these symbols not to be internalized by LTO. But will
/// not be added to `symbols.o`. Currently there are only builtin functions.
pub used_compiler: bool,
}

#[derive(Eq, PartialEq, Debug, Copy, Clone, TyEncodable, TyDecodable, HashStable)]
Expand Down
1 change: 1 addition & 0 deletions src/tools/miri/src/bin/miri.rs
Original file line number Diff line number Diff line change
Expand Up @@ -165,6 +165,7 @@ impl rustc_driver::Callbacks for MiriBeRustCompilerCalls {
level: SymbolExportLevel::C,
kind: SymbolExportKind::Text,
used: false,
used_compiler: false,
},
))
}),
Expand Down
7 changes: 7 additions & 0 deletions tests/run-make/no-builtins-symbols/Makefile
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
include ../tools.mk

# only-x86_64-unknown-linux-gnu

all:
$(RUSTC) main.rs -o $(TMPDIR)/main
[ "$$("$(LLVM_BIN_DIR)"/llvm-nm -U $(TMPDIR)/main | grep -c __fixunssfti)" -eq "0" ]
1 change: 1 addition & 0 deletions tests/run-make/no-builtins-symbols/main.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
fn main() {}

0 comments on commit 3a9fb17

Please sign in to comment.