Auto merge of rust-lang#113923 - DianQK:restore-no-builtins-lto, r=pn…

…kfelix

Restore `#![no_builtins]` crates participation in LTO.

After rust-lang#113716, we can make `#![no_builtins]` crates participate in LTO again.

`#![no_builtins]` with LTO does not result in undefined references to the error. I believe this type of issue won't happen again. ＼（＾▽＾）／

I will test the following issues later to verify. The task format is `Fixes {issue} {nightly-2023-07-20 result} {PR rust-lang#113923 result}`.

- [x] Fixes rust-lang#72140. ❌ ✅
- [x] Fixes rust-lang#112245. ❌ ✅
- [x] Fixes rust-lang#110606. ❌ ✅
- [ ] Fixes rust-lang#105734.
- [ ] Fixes rust-lang#96486.
- [ ] Fixes rust-lang#108853.
- [x] Fixes rust-lang/compiler-builtins#347. ❌ ✅
- [ ] Fixes rust-lang#108893.
- [ ] Fixes rust-lang#78744. Fixes rust-lang#91158. Fixes rust-lang/cargo#10118.

 The `nightly-2023-07-20` version does not always reproduce problems due to changes in compiler-builtins, core, and user code. That's why this issue recurs and disappears.
Some issues were not tested due to the difficulty of reproducing them.

r? pnkfelix

cc `@bjorn3` `@japaric` `@alexcrichton` `@Amanieu`

compiler/rustc_codegen_llvm/src/back/write.rs

@@ -563,7 +563,6 @@ 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()),
@@ -678,15 +677,14 @@ 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
             F: FnOnce(&'ll mut PassManager<'ll>) -> R,
         {
             let cpm = llvm::LLVMCreatePassManager();
             llvm::LLVMAddAnalysisPasses(tm, cpm);
-            llvm::LLVMRustAddLibraryInfo(cpm, llmod, no_builtins);
+            llvm::LLVMRustAddLibraryInfo(cpm, llmod);
             f(cpm)
         }
 
@@ -789,7 +787,7 @@ pub(crate) unsafe fn codegen(
             } else {
                 llmod
             };
-            with_codegen(tm, llmod, config.no_builtins, |cpm| {
+            with_codegen(tm, llmod, |cpm| {
                 write_output_file(
                     diag_handler,
                     tm,
@@ -824,7 +822,7 @@ pub(crate) unsafe fn codegen(
                     (_, SplitDwarfKind::Split) => Some(dwo_out.as_path()),
                 };
 
-                with_codegen(tm, llmod, config.no_builtins, |cpm| {
+                with_codegen(tm, llmod, |cpm| {
                     write_output_file(
                         diag_handler,
                         tm,

compiler/rustc_codegen_llvm/src/llvm/ffi.rs

@@ -2139,13 +2139,8 @@ extern "C" {
         ArgsCstrBuff: *const c_char,
         ArgsCstrBuffLen: usize,
     ) -> *mut TargetMachine;
-
     pub fn LLVMRustDisposeTargetMachine(T: *mut TargetMachine);
-    pub fn LLVMRustAddLibraryInfo<'a>(
-        PM: &PassManager<'a>,
-        M: &'a Module,
-        DisableSimplifyLibCalls: bool,
-    );
+    pub fn LLVMRustAddLibraryInfo<'a>(PM: &PassManager<'a>, M: &'a Module);
     pub fn LLVMRustWriteOutputFile<'a>(
         T: &'a TargetMachine,
         PM: &PassManager<'a>,
@@ -2167,7 +2162,6 @@ extern "C" {
         UnrollLoops: bool,
         SLPVectorize: bool,
         LoopVectorize: bool,
-        DisableSimplifyLibCalls: bool,
         EmitLifetimeMarkers: bool,
         SanitizerOptions: Option<&SanitizerOptions>,
         PGOGenPath: *const c_char,

compiler/rustc_codegen_ssa/src/back/link.rs

@@ -518,8 +518,7 @@ fn link_staticlib<'a>(
         &codegen_results.crate_info,
         Some(CrateType::Staticlib),
         &mut |cnum, path| {
-            let lto = are_upstream_rust_objects_already_included(sess)
-                && !ignored_for_lto(sess, &codegen_results.crate_info, cnum);
+            let lto = are_upstream_rust_objects_already_included(sess);
 
             let native_libs = codegen_results.crate_info.native_libraries[&cnum].iter();
             let relevant = native_libs.clone().filter(|lib| relevant_lib(sess, &lib));
@@ -1258,24 +1257,6 @@ fn link_sanitizer_runtime(sess: &Session, linker: &mut dyn Linker, name: &str) {
     }
 }
 
-/// 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(
@@ -2741,10 +2722,6 @@ 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.
@@ -2770,9 +2747,7 @@ fn add_static_crate<'a>(
         cmd.link_rlib(&rlib_path);
     };
 
-    if !are_upstream_rust_objects_already_included(sess)
-        || ignored_for_lto(sess, &codegen_results.crate_info, cnum)
-    {
+    if !are_upstream_rust_objects_already_included(sess) {
         link_upstream(cratepath);
         return;
     }
@@ -2786,8 +2761,6 @@ 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(
@@ -2804,9 +2777,8 @@ 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. 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 {
+                // LTO module.
+                if upstream_rust_objects_already_included && is_rust_object {
                     return true;
                 }
 

compiler/rustc_codegen_ssa/src/back/write.rs

@@ -148,23 +148,12 @@ impl ModuleConfig {
 
         let emit_obj = if !should_emit_obj {
             EmitObj::None
-        } else if sess.target.obj_is_bitcode
-            || (sess.opts.cg.linker_plugin_lto.enabled() && !no_builtins)
-        {
+        } else if sess.target.obj_is_bitcode || sess.opts.cg.linker_plugin_lto.enabled() {
             // 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)
@@ -1037,9 +1026,6 @@ 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()));
     }));
 

compiler/rustc_codegen_ssa/src/base.rs

@@ -891,9 +891,7 @@ impl 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, 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.
+        // (*) Native libs are not glued, and we assume that they cannot define weak lang items.
         let target = &tcx.sess.target;
         if !are_upstream_rust_objects_already_included(tcx.sess) {
             let missing_weak_lang_items: FxHashSet<Symbol> = info

compiler/rustc_llvm/llvm-wrapper/PassWrapper.cpp

@@ -535,12 +535,9 @@ 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,
-                                       bool DisableSimplifyLibCalls) {
+extern "C" void LLVMRustAddLibraryInfo(LLVMPassManagerRef PMR, LLVMModuleRef M) {
   Triple TargetTriple(unwrap(M)->getTargetTriple());
   TargetLibraryInfoImpl TLII(TargetTriple);
-  if (DisableSimplifyLibCalls)
-    TLII.disableAllFunctions();
   unwrap(PMR)->add(new TargetLibraryInfoWrapperPass(TLII));
 }
 
@@ -707,7 +704,7 @@ LLVMRustOptimize(
     bool IsLinkerPluginLTO,
     bool NoPrepopulatePasses, bool VerifyIR, bool UseThinLTOBuffers,
     bool MergeFunctions, bool UnrollLoops, bool SLPVectorize, bool LoopVectorize,
-    bool DisableSimplifyLibCalls, bool EmitLifetimeMarkers,
+    bool EmitLifetimeMarkers,
     LLVMRustSanitizerOptions *SanitizerOptions,
     const char *PGOGenPath, const char *PGOUsePath,
     bool InstrumentCoverage, const char *InstrProfileOutput,
@@ -813,8 +810,6 @@ 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);
@@ -1125,6 +1120,102 @@ extern "C" void LLVMRustPrintPasses() {
   PB.printPassNames(outs());
 }
 
+// from https://github.com/llvm/llvm-project/blob/7021182d6b43de9488ab70de626192ce70b3a4a6/llvm/lib/Object/IRSymtab.cpp#L48-L57
+static const char *PreservedLibcallSymbols[] = {
+#define HANDLE_LIBCALL(code, name) name,
+#include "llvm/IR/RuntimeLibcalls.def"
+#undef HANDLE_LIBCALL
+  // RuntimeLibcalls.def missing symbols.
+  "__ctzsi2",
+  "__ctzdi2",
+  "__ctzti2",
+  "__ffssi2",
+  "__ffsdi2",
+  "__ffsti2",
+  "__paritysi2",
+  "__paritydi2",
+  "__parityti2",
+  "__popcountsi2",
+  "__popcountdi2",
+  "__popcountti2",
+  "__bswapsi2",
+  "__bswapdi2",
+  "__negti2",
+  "__udivmoddi4",
+  "__udivmodti4",
+  "__udivmodsi4",
+  "__divmodsi4",
+  "__divmoddi4",
+  "__divmodti4",
+  "__absvsi2",
+  "__absvdi2",
+  "__absvti2",
+  "__negvsi2",
+  "__negvdi2",
+  "__negvti2",
+  "__addvsi3",
+  "__addvdi3",
+  "__addvti3",
+  "__subvsi3",
+  "__subvdi3",
+  "__subvti3",
+  "__mulvsi3",
+  "__mulvdi3",
+  "__mulvti3",
+  "__cmpdi2",
+  "__cmpti2",
+  "__ucmpdi2",
+  "__ucmpti2",
+  "__mulsc3",
+  "__muldc3",
+  "__mulxc3",
+  "__multc3",
+  "__divsc3",
+  "__divdc3",
+  "__divxc3",
+  "__divtc3",
+  "__clear_cache",
+  "__enable_execute_stack",
+  "__gcc_personality_v0",
+  "__eprintf",
+  "__emutls_get_address",
+  "__trampoline_setup",
+  "__addsf3vfp",
+  "__adddf3vfp",
+  "__divsf3vfp",
+  "__divdf3vfp",
+  "__eqsf2vfp",
+  "__eqdf2vfp",
+  "__extendsfdf2vfp",
+  "__fixdfsivfp",
+  "__fixsfsivfp",
+  "__fixunssfsivfp",
+  "__fixunsdfsivfp",
+  "__floatsidfvfp",
+  "__floatsisfvfp",
+  "__floatunssidfvfp",
+  "__floatunssisfvfp",
+  "__gedf2vfp",
+  "__gesf2vfp",
+  "__gtdf2vfp",
+  "__gtsf2vfp",
+  "__ledf2vfp",
+  "__lesf2vfp",
+  "__ltdf2vfp",
+  "__ltsf2vfp",
+  "__muldf3vfp",
+  "__mulsf3vfp",
+  "__nedf2vfp",
+  "__negdf2vfp",
+  "__negsf2vfp",
+  "__negsf2vfp",
+  "__subdf3vfp",
+  "__subsf3vfp",
+  "__truncdfsf2vfp",
+  "__unorddf2vfp",
+  "__unordsf2vfp",
+};
+
 extern "C" void LLVMRustRunRestrictionPass(LLVMModuleRef M, char **Symbols,
                                            size_t Len) {
   auto PreserveFunctions = [=](const GlobalValue &GV) {
@@ -1140,7 +1231,7 @@ extern "C" void LLVMRustRunRestrictionPass(LLVMModuleRef M, char **Symbols,
         return true;
       }
     }
-    return false;
+    return llvm::is_contained(PreservedLibcallSymbols, GV.getName());
   };
 
   internalizeModule(*unwrap(M), PreserveFunctions);
@@ -1298,6 +1389,12 @@ LLVMRustCreateThinLTOData(LLVMRustThinLTOModule *modules,
     auto GUID = GlobalValue::getGUID(preserved_symbols[i]);
     Ret->GUIDPreservedSymbols.insert(GUID);
   }
+  for (int i = 0; i < sizeof(PreservedLibcallSymbols) / sizeof(PreservedLibcallSymbols[0]); i++) {
+    if (auto *PreservedLibcallSymbol = PreservedLibcallSymbols[i]) {
+      auto GUID = GlobalValue::getGUID(PreservedLibcallSymbol);
+      Ret->GUIDPreservedSymbols.insert(GUID);
+    }
+  }
 
   // Collect the import/export lists for all modules from the call-graph in the
   // combined index

tests/run-make/no-builtins-lto/Makefile

@@ -1,9 +1,15 @@
 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:
-	# 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'
+	$(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

tests/run-make/no-builtins-lto/filecheck.lto.txt

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