Rollup merge of #119885 - DianQK:revert-pr-113923, r=petrochenkov

Revert #113923

Per [#t-compiler/meetings > [weekly] 2024-01-11](https://rust-lang.zulipchat.com/#narrow/stream/238009-t-compiler.2Fmeetings/topic/.5Bweekly.5D.202024-01-11) discussion, revert #113923. Also revert associated #118568.

The PR #113923 causes the regression issue #118609. We need more time to find a proper solution.

Discussions start at [412365838](https://rust-lang.zulipchat.com/#narrow/stream/238009-t-compiler.2Fmeetings/topic/.5Bweekly.5D.202024-01-11/near/412365838) and continue to [412369643](https://rust-lang.zulipchat.com/#narrow/stream/238009-t-compiler.2Fmeetings/topic/.5Bweekly.5D.202024-01-11/near/412369643).

Fixes #118609.

r? compiler
This commit is contained in:
Guillaume Gomez 2024-01-12 15:16:58 +01:00 committed by GitHub
commit dafbe17a02
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
21 changed files with 90 additions and 160 deletions

View File

@ -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 || info.used_compiler {
if info.level.is_below_threshold(export_threshold) || info.used {
Some(CString::new(name.as_str()).unwrap())
} else {
None

View File

@ -569,6 +569,7 @@ pub(crate) unsafe fn llvm_optimize(
unroll_loops,
config.vectorize_slp,
config.vectorize_loop,
config.no_builtins,
config.emit_lifetime_markers,
sanitizer_options.as_ref(),
pgo_gen_path.as_ref().map_or(std::ptr::null(), |s| s.as_ptr()),
@ -677,6 +678,7 @@ pub(crate) unsafe fn codegen(
unsafe fn with_codegen<'ll, F, R>(
tm: &'ll llvm::TargetMachine,
llmod: &'ll llvm::Module,
no_builtins: bool,
f: F,
) -> R
where
@ -684,7 +686,7 @@ unsafe fn with_codegen<'ll, F, R>(
{
let cpm = llvm::LLVMCreatePassManager();
llvm::LLVMAddAnalysisPasses(tm, cpm);
llvm::LLVMRustAddLibraryInfo(cpm, llmod);
llvm::LLVMRustAddLibraryInfo(cpm, llmod, no_builtins);
f(cpm)
}
@ -785,7 +787,7 @@ extern "C" fn demangle_callback(
} else {
llmod
};
with_codegen(tm, llmod, |cpm| {
with_codegen(tm, llmod, config.no_builtins, |cpm| {
write_output_file(
dcx,
tm,
@ -820,7 +822,7 @@ extern "C" fn demangle_callback(
(_, SplitDwarfKind::Split) => Some(dwo_out.as_path()),
};
with_codegen(tm, llmod, |cpm| {
with_codegen(tm, llmod, config.no_builtins, |cpm| {
write_output_file(
dcx,
tm,

View File

@ -2173,8 +2173,13 @@ pub fn LLVMRustCreateTargetMachine(
ArgsCstrBuff: *const c_char,
ArgsCstrBuffLen: usize,
) -> *mut TargetMachine;
pub fn LLVMRustDisposeTargetMachine(T: *mut TargetMachine);
pub fn LLVMRustAddLibraryInfo<'a>(PM: &PassManager<'a>, M: &'a Module);
pub fn LLVMRustAddLibraryInfo<'a>(
PM: &PassManager<'a>,
M: &'a Module,
DisableSimplifyLibCalls: bool,
);
pub fn LLVMRustWriteOutputFile<'a>(
T: &'a TargetMachine,
PM: &PassManager<'a>,
@ -2196,6 +2201,7 @@ pub fn LLVMRustOptimize<'a>(
UnrollLoops: bool,
SLPVectorize: bool,
LoopVectorize: bool,
DisableSimplifyLibCalls: bool,
EmitLifetimeMarkers: bool,
SanitizerOptions: Option<&SanitizerOptions>,
PGOGenPath: *const c_char,

View File

@ -270,14 +270,8 @@ pub fn each_linked_rlib(
for &cnum in crates {
match fmts.get(cnum.as_usize() - 1) {
Some(&Linkage::NotLinked | &Linkage::Dynamic) => continue,
Some(&Linkage::IncludedFromDylib) => {
// We always link crate `compiler_builtins` statically. When enabling LTO, we include it as well.
if info.compiler_builtins != Some(cnum) {
continue;
}
}
Some(&Linkage::Static) => {}
Some(&Linkage::NotLinked | &Linkage::Dynamic | &Linkage::IncludedFromDylib) => continue,
Some(_) => {}
None => return Err(errors::LinkRlibError::MissingFormat),
}
let crate_name = info.crate_name[&cnum];
@ -526,7 +520,8 @@ fn link_staticlib<'a>(
&codegen_results.crate_info,
Some(CrateType::Staticlib),
&mut |cnum, path| {
let lto = are_upstream_rust_objects_already_included(sess);
let lto = are_upstream_rust_objects_already_included(sess)
&& !ignored_for_lto(sess, &codegen_results.crate_info, cnum);
let native_libs = codegen_results.crate_info.native_libraries[&cnum].iter();
let relevant = native_libs.clone().filter(|lib| relevant_lib(sess, lib));
@ -1277,6 +1272,24 @@ fn find_sanitizer_runtime(sess: &Session, filename: &str) -> PathBuf {
}
}
/// Returns a boolean indicating whether the specified crate should be ignored
/// during LTO.
///
/// Crates ignored during LTO are not lumped together in the "massive object
/// file" that we create and are linked in their normal rlib states. See
/// comments below for what crates do not participate in LTO.
///
/// It's unusual for a crate to not participate in LTO. Typically only
/// compiler-specific and unstable crates have a reason to not participate in
/// LTO.
pub fn ignored_for_lto(sess: &Session, info: &CrateInfo, cnum: CrateNum) -> bool {
// If our target enables builtin function lowering in LLVM then the
// crates providing these functions don't participate in LTO (e.g.
// no_builtins or compiler builtins crates).
!sess.target.no_builtins
&& (info.compiler_builtins == Some(cnum) || info.is_no_builtins.contains(&cnum))
}
/// This functions tries to determine the appropriate linker (and corresponding LinkerFlavor) to use
pub fn linker_and_flavor(sess: &Session) -> (PathBuf, LinkerFlavor) {
fn infer_from(
@ -2742,6 +2755,10 @@ fn rehome_sysroot_lib_dir<'a>(sess: &'a Session, lib_dir: &Path) -> PathBuf {
// symbols). We must continue to include the rest of the rlib, however, as
// it may contain static native libraries which must be linked in.
//
// (*) Crates marked with `#![no_builtins]` don't participate in LTO and
// their bytecode wasn't included. The object files in those libraries must
// still be passed to the linker.
//
// Note, however, that if we're not doing LTO we can just pass the rlib
// blindly to the linker (fast) because it's fine if it's not actually
// included as we're at the end of the dependency chain.
@ -2767,7 +2784,9 @@ fn add_static_crate<'a>(
cmd.link_rlib(&rlib_path);
};
if !are_upstream_rust_objects_already_included(sess) {
if !are_upstream_rust_objects_already_included(sess)
|| ignored_for_lto(sess, &codegen_results.crate_info, cnum)
{
link_upstream(cratepath);
return;
}
@ -2781,6 +2800,8 @@ fn add_static_crate<'a>(
let canonical_name = name.replace('-', "_");
let upstream_rust_objects_already_included =
are_upstream_rust_objects_already_included(sess);
let is_builtins =
sess.target.no_builtins || !codegen_results.crate_info.is_no_builtins.contains(&cnum);
let mut archive = archive_builder_builder.new_archive_builder(sess);
if let Err(error) = archive.add_archive(
@ -2797,8 +2818,9 @@ fn add_static_crate<'a>(
// If we're performing LTO and this is a rust-generated object
// file, then we don't need the object file as it's part of the
// LTO module.
if upstream_rust_objects_already_included && is_rust_object {
// LTO module. Note that `#![no_builtins]` is excluded from LTO,
// though, so we let that object file slide.
if upstream_rust_objects_already_included && is_rust_object && is_builtins {
return true;
}

View File

@ -54,8 +54,8 @@ fn reachable_non_generics_provider(tcx: TyCtxt<'_>, _: LocalCrate) -> DefIdMap<S
// export level, however, as they're just implementation details.
// Down below we'll hardwire all of the symbols to the `Rust` export
// level instead.
let is_compiler_builtins = tcx.is_compiler_builtins(LOCAL_CRATE);
let special_runtime_crate = tcx.is_panic_runtime(LOCAL_CRATE) || is_compiler_builtins;
let special_runtime_crate =
tcx.is_panic_runtime(LOCAL_CRATE) || tcx.is_compiler_builtins(LOCAL_CRATE);
let mut reachable_non_generics: DefIdMap<_> = tcx
.reachable_set(())
@ -105,14 +105,8 @@ 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)
&& codegen_attrs.flags.contains(CodegenFnAttrFlags::NO_MANGLE);
let used = name == "rust_eh_personality";
let export_level = if special_runtime_crate {
@ -120,6 +114,7 @@ fn reachable_non_generics_provider(tcx: TyCtxt<'_>, _: LocalCrate) -> DefIdMap<S
} 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())),
@ -139,7 +134,6 @@ 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)
})
@ -152,7 +146,6 @@ fn reachable_non_generics_provider(tcx: TyCtxt<'_>, _: LocalCrate) -> DefIdMap<S
level: SymbolExportLevel::C,
kind: SymbolExportKind::Data,
used: false,
used_compiler: false,
},
);
}
@ -201,7 +194,6 @@ fn exported_symbols_provider_local(
level: info.level,
kind: SymbolExportKind::Text,
used: info.used,
used_compiler: false,
},
)
})
@ -218,7 +210,6 @@ fn exported_symbols_provider_local(
level: SymbolExportLevel::C,
kind: SymbolExportKind::Text,
used: false,
used_compiler: false,
},
));
}
@ -238,7 +229,6 @@ fn exported_symbols_provider_local(
level: SymbolExportLevel::Rust,
kind: SymbolExportKind::Text,
used: false,
used_compiler: false,
},
));
}
@ -251,7 +241,6 @@ fn exported_symbols_provider_local(
level: SymbolExportLevel::Rust,
kind: SymbolExportKind::Data,
used: false,
used_compiler: false,
},
))
}
@ -271,7 +260,6 @@ fn exported_symbols_provider_local(
level: SymbolExportLevel::C,
kind: SymbolExportKind::Data,
used: false,
used_compiler: false,
},
)
}));
@ -297,7 +285,6 @@ fn exported_symbols_provider_local(
level: SymbolExportLevel::C,
kind: SymbolExportKind::Data,
used: false,
used_compiler: false,
},
)
}));
@ -315,7 +302,6 @@ fn exported_symbols_provider_local(
level: SymbolExportLevel::C,
kind: SymbolExportKind::Data,
used: true,
used_compiler: false,
},
));
}
@ -356,7 +342,6 @@ fn exported_symbols_provider_local(
level: SymbolExportLevel::Rust,
kind: SymbolExportKind::Text,
used: false,
used_compiler: false,
},
));
}
@ -373,7 +358,6 @@ fn exported_symbols_provider_local(
level: SymbolExportLevel::Rust,
kind: SymbolExportKind::Text,
used: false,
used_compiler: false,
},
));
}

View File

@ -148,12 +148,23 @@ macro_rules! if_regular {
let emit_obj = if !should_emit_obj {
EmitObj::None
} else if sess.target.obj_is_bitcode || sess.opts.cg.linker_plugin_lto.enabled() {
} else if sess.target.obj_is_bitcode
|| (sess.opts.cg.linker_plugin_lto.enabled() && !no_builtins)
{
// This case is selected if the target uses objects as bitcode, or
// if linker plugin LTO is enabled. In the linker plugin LTO case
// the assumption is that the final link-step will read the bitcode
// and convert it to object code. This may be done by either the
// native linker or rustc itself.
//
// Note, however, that the linker-plugin-lto requested here is
// explicitly ignored for `#![no_builtins]` crates. These crates are
// specifically ignored by rustc's LTO passes and wouldn't work if
// loaded into the linker. These crates define symbols that LLVM
// lowers intrinsics to, and these symbol dependencies aren't known
// until after codegen. As a result any crate marked
// `#![no_builtins]` is assumed to not participate in LTO and
// instead goes on to generate object code.
EmitObj::Bitcode
} else if need_bitcode_in_object(tcx) {
EmitObj::ObjectCode(BitcodeSection::Full)
@ -1023,6 +1034,9 @@ fn start_executing_work<B: ExtraBackendMethods>(
let mut each_linked_rlib_for_lto = Vec::new();
drop(link::each_linked_rlib(crate_info, None, &mut |cnum, path| {
if link::ignored_for_lto(sess, crate_info, cnum) {
return;
}
each_linked_rlib_for_lto.push((cnum, path.to_path_buf()));
}));

View File

@ -859,6 +859,7 @@ pub fn new(tcx: TyCtxt<'_>, target_cpu: String) -> CrateInfo {
local_crate_name,
compiler_builtins,
profiler_runtime: None,
is_no_builtins: Default::default(),
native_libraries: Default::default(),
used_libraries: tcx.native_libraries(LOCAL_CRATE).iter().map(Into::into).collect(),
crate_name: Default::default(),
@ -885,6 +886,9 @@ pub fn new(tcx: TyCtxt<'_>, target_cpu: String) -> CrateInfo {
if tcx.is_profiler_runtime(cnum) {
info.profiler_runtime = Some(cnum);
}
if tcx.is_no_builtins(cnum) {
info.is_no_builtins.insert(cnum);
}
}
// Handle circular dependencies in the standard library.
@ -892,7 +896,9 @@ pub fn new(tcx: TyCtxt<'_>, target_cpu: String) -> CrateInfo {
// If global LTO is enabled then almost everything (*) is glued into a single object file,
// so this logic is not necessary and can cause issues on some targets (due to weak lang
// item symbols being "privatized" to that object file), so we disable it.
// (*) Native libs are not glued, and we assume that they cannot define weak lang items.
// (*) Native libs, and `#[compiler_builtins]` and `#[no_builtins]` crates are not glued,
// and we assume that they cannot define weak lang items. This is not currently enforced
// by the compiler, but that's ok because all this stuff is unstable anyway.
let target = &tcx.sess.target;
if !are_upstream_rust_objects_already_included(tcx.sess) {
let missing_weak_lang_items: FxHashSet<Symbol> = info

View File

@ -25,7 +25,7 @@
extern crate rustc_middle;
use rustc_ast as ast;
use rustc_data_structures::fx::FxHashMap;
use rustc_data_structures::fx::{FxHashMap, FxHashSet};
use rustc_data_structures::sync::Lrc;
use rustc_hir::def_id::CrateNum;
use rustc_middle::dep_graph::WorkProduct;
@ -158,6 +158,7 @@ pub struct CrateInfo {
pub local_crate_name: Symbol,
pub compiler_builtins: Option<CrateNum>,
pub profiler_runtime: Option<CrateNum>,
pub is_no_builtins: FxHashSet<CrateNum>,
pub native_libraries: FxHashMap<CrateNum, Vec<NativeLib>>,
pub crate_name: FxHashMap<CrateNum, Symbol>,
pub used_libraries: Vec<NativeLib>,

View File

@ -531,9 +531,12 @@ extern "C" void LLVMRustDisposeTargetMachine(LLVMTargetMachineRef TM) {
// Unfortunately, the LLVM C API doesn't provide a way to create the
// TargetLibraryInfo pass, so we use this method to do so.
extern "C" void LLVMRustAddLibraryInfo(LLVMPassManagerRef PMR, LLVMModuleRef M) {
extern "C" void LLVMRustAddLibraryInfo(LLVMPassManagerRef PMR, LLVMModuleRef M,
bool DisableSimplifyLibCalls) {
Triple TargetTriple(unwrap(M)->getTargetTriple());
TargetLibraryInfoImpl TLII(TargetTriple);
if (DisableSimplifyLibCalls)
TLII.disableAllFunctions();
unwrap(PMR)->add(new TargetLibraryInfoWrapperPass(TLII));
}
@ -700,7 +703,7 @@ LLVMRustOptimize(
bool IsLinkerPluginLTO,
bool NoPrepopulatePasses, bool VerifyIR, bool UseThinLTOBuffers,
bool MergeFunctions, bool UnrollLoops, bool SLPVectorize, bool LoopVectorize,
bool EmitLifetimeMarkers,
bool DisableSimplifyLibCalls, bool EmitLifetimeMarkers,
LLVMRustSanitizerOptions *SanitizerOptions,
const char *PGOGenPath, const char *PGOUsePath,
bool InstrumentCoverage, const char *InstrProfileOutput,
@ -800,6 +803,8 @@ LLVMRustOptimize(
Triple TargetTriple(TheModule->getTargetTriple());
std::unique_ptr<TargetLibraryInfoImpl> TLII(new TargetLibraryInfoImpl(TargetTriple));
if (DisableSimplifyLibCalls)
TLII->disableAllFunctions();
FAM.registerPass([&] { return TargetLibraryAnalysis(*TLII); });
PB.registerModuleAnalyses(MAM);

View File

@ -35,12 +35,7 @@ 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)]

View File

@ -165,7 +165,6 @@ fn config(&mut self, config: &mut Config) {
level: SymbolExportLevel::C,
kind: SymbolExportKind::Text,
used: false,
used_compiler: false,
},
))
}),

View File

@ -1,15 +1,9 @@
include ../tools.mk
# only-x86_64
# We want to check that `no_builtins` is correctly participating in LTO.
# First, verify that the `foo::foo` symbol can be found when linking.
# Next, verify that `memcpy` can be customized using `no_builtins` under LTO.
# Others will use the built-in memcpy.
all:
$(RUSTC) -C linker-plugin-lto -C opt-level=2 -C debuginfo=0 foo.rs
$(RUSTC) -C linker-plugin-lto -C opt-level=2 -C debuginfo=0 no_builtins.rs
$(RUSTC) main.rs -C lto -C opt-level=2 -C debuginfo=0 -C save-temps -C metadata=1 -C codegen-units=1
"$(LLVM_BIN_DIR)"/llvm-dis $(TMPDIR)/main.main.*-cgu.0.rcgu.lto.input.bc -o $(TMPDIR)/lto.ll
cat "$(TMPDIR)"/lto.ll | "$(LLVM_FILECHECK)" filecheck.lto.txt
# Compile a `#![no_builtins]` rlib crate
$(RUSTC) no_builtins.rs
# Build an executable that depends on that crate using LTO. The no_builtins crate doesn't
# participate in LTO, so its rlib must be explicitly linked into the final binary. Verify this by
# grepping the linker arguments.
$(RUSTC) main.rs -C lto --print link-args | $(CGREP) 'libno_builtins.rlib'

View File

@ -1,17 +0,0 @@
CHECK: define{{.*}} void @bar
CHECK-NEXT: call void @no_builtins
CHECK-NEXT: call void @llvm.memcpy
CHECK: define{{.*}} i32 @main
CHECK: call void @bar
CHECK: define{{.*}} void @foo
CHECK-NEXT: call void @llvm.memcpy
CHECK: define{{.*}} void @no_builtins
CHECK-SAME: #[[ATTR:[0-9]+]] {
CHECK: call void @foo
CHECK-NEXT: call{{.*}} @memcpy
CHECK: attributes #[[ATTR]]
CHECK-SAME: no-builtins

View File

@ -1,33 +0,0 @@
#![feature(lang_items, no_core)]
#![no_std]
#![no_core]
#![crate_type = "lib"]
#[inline(never)]
#[no_mangle]
pub unsafe fn foo(dest: *mut u8, src: *const u8) {
// should call `@llvm.memcpy`.
memcpy(dest, src, 1024);
}
#[no_mangle]
#[inline(never)]
pub unsafe extern "C" fn memcpy(dest: *mut u8, src: *const u8, _n: usize) -> *mut u8 {
*dest = 0;
return src as *mut u8;
}
#[lang = "sized"]
trait Sized {}
#[lang = "copy"]
trait Copy {}
impl Copy for *mut u8 {}
impl Copy for *const u8 {}
#[lang = "drop_in_place"]
#[allow(unconditional_recursion)]
pub unsafe fn drop_in_place<T: ?Sized>(to_drop: *mut T) {
// Code here does not matter - this is replaced by the
// real drop glue by the compiler.
drop_in_place(to_drop);
}

View File

@ -1,29 +1,3 @@
#![feature(no_core, start, lang_items)]
#![no_std]
// We use `no_core` to reduce the LTO products is small enough.
#![no_core]
extern crate no_builtins;
extern crate foo;
#[cfg_attr(unix, link(name = "c"))]
#[cfg_attr(target_env = "msvc", link(name = "msvcrt"))]
extern "C" {}
#[start]
fn main(_: isize, p: *const *const u8) -> isize {
// Make sure the symbols are retained.
unsafe { bar(*p as *mut u8, *p); }
0
}
#[no_mangle]
#[inline(never)]
pub unsafe extern "C" fn bar(dest: *mut u8, src: *const u8) {
no_builtins::no_builtins(dest, src);
// should call `@llvm.memcpy`
foo::memcpy(dest, src, 1024);
}
#[lang = "eh_personality"]
fn eh_personality() {}
fn main() {}

View File

@ -1,15 +1,2 @@
#![feature(lang_items, no_core)]
#![no_std]
#![no_core]
#![crate_type = "lib"]
#![no_builtins]
extern crate foo;
#[no_mangle]
pub unsafe fn no_builtins(dest: *mut u8, src: *const u8) {
// There should be no "undefined reference to `foo::foo'".
foo::foo(dest, src);
// should call `@memcpy` instead of `@llvm.memcpy`.
foo::memcpy(dest, src, 1024);
}

View File

@ -1,7 +0,0 @@
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" ]

View File

@ -1 +0,0 @@
fn main() {}

View File

@ -8,8 +8,7 @@ fn my_panic(_info: &core::panic::PanicInfo) -> ! {
#[no_mangle]
pub fn multer(a: i128, b: i128) -> i128 {
// Trigger usage of the __multi3 compiler intrinsic which then leads to an imported function
// such as panic or __multi3 (externally defined) in case of a bug. We verify that
// no imports exist in our verifier.
// Trigger usage of the __multi3 compiler intrinsic which then leads to an imported
// panic function in case of a bug. We verify that no imports exist in our verifier.
a * b
}