LTO: Mark undefined module inline asm symbols as visible outside of ThinLTO.

[pcc]

Summary

Fixes PR32798.

[davide]

llvm/lib/Object/IRSymtab.cpp

166–168 ↗

(On Diff #96779)

With this patch in place, do we still need the call to collectAsmUndefined in the ThinLTO backend? (I understand we need it for LTO, still)

[pcc]

• Stop calling handleAsmUndefinedRefs

[pcc]

llvm/lib/Object/IRSymtab.cpp

166–168 ↗

(On Diff #96779)

Good point. I actually think we can remove it from both because preventing internalization should be sufficient to keep the symbol alive.

[davide]

llvm/lib/Object/IRSymtab.cpp

166–168 ↗

(On Diff #96779)

You probably already noticed, but there may be some test in lld that needs updating (so you can catch it before the bots start yelling).

[pcc]

llvm/lib/Object/IRSymtab.cpp

166–168 ↗

(On Diff #96779)

Yes, that was lld/test/ELF/lto/asmundef.ll, which I already updated :)

[davide]

The LTO bits look fine to me. I think this is also the correct fix for ThinLTO but Teresa should confirm

llvm/lib/Object/IRSymtab.cpp

166–168 ↗

(On Diff #96779)

Oh, was looking at a stale revision.

[davide]

(also update the revision title before committing)

[tejohnson]

LGTM

[mehdi_amini]

Won't this disable *linker* dead-stripping?
I don't understand the motivation for this change right now?

[pcc]

In D32544#738497, @mehdi_amini wrote:

Won't this disable *linker* dead-stripping?

I don't think so. LTO will only internalize a global if the linker tells it that is not a GC root. This change disables internalization for some entities that would otherwise be internalized, but linker dead stripping is not inhibited because even if the symbol had external linkage, it would still not be a GC root.

I don't understand the motivation for this change right now?

See PR32798.

lld/trunk/test/ELF/lto/asmundef.ll

define void @foo() {
   ret void
 }
 
 define void @_start() {
   call void @patatino()
   ret void
 }
 
-; CHECK: @llvm.compiler.used = appending global [1 x i8*] [i8* bitcast (void ()* @foo to i8*)], section "llvm.metadata"
-; CHECK: define internal void @foo
+; CHECK: define void @foo
 

llvm/trunk/lib/LTO/LTO.cpp

if (Res.VisibleToRegularObj || Sym.isUsed() ||
     GlobalRes.Partition = GlobalResolution::External;
   } else
     // First recorded reference, save the current partition.
     GlobalRes.Partition = Partition;
 
   // Flag as visible outside of ThinLTO if visible from a regular object or
   // if this is a reference in the regular LTO partition.
   GlobalRes.VisibleOutsideThinLTO |=
-      (Res.VisibleToRegularObj || (Partition == GlobalResolution::RegularLTO));
+      (Res.VisibleToRegularObj || Sym.isUsed() ||
+       Partition == GlobalResolution::RegularLTO);
 }
 
 static void writeToResolutionFile(raw_ostream &OS, InputFile *Input,
                                   ArrayRef<SymbolResolution> Res) {
   StringRef Path = Input->getName();
   OS << Path << '\n';
   auto ResI = Res.begin();
   for (const InputFile::Symbol &Sym : Input->symbols()) {

llvm/trunk/lib/LTO/LTOBackend.cpp

 #include "llvm/Analysis/TargetLibraryInfo.h"
 #include "llvm/Analysis/TargetTransformInfo.h"
 #include "llvm/Bitcode/BitcodeReader.h"
 #include "llvm/Bitcode/BitcodeWriter.h"
 #include "llvm/IR/LegacyPassManager.h"
 #include "llvm/IR/PassManager.h"
 #include "llvm/IR/Verifier.h"
 #include "llvm/LTO/LTO.h"
-#include "llvm/LTO/legacy/UpdateCompilerUsed.h"
 #include "llvm/MC/SubtargetFeature.h"
 #include "llvm/Object/ModuleSymbolTable.h"
 #include "llvm/Passes/PassBuilder.h"
 #include "llvm/Support/Error.h"
 #include "llvm/Support/FileSystem.h"
 #include "llvm/Support/TargetRegistry.h"
 #include "llvm/Support/ThreadPool.h"
 #include "llvm/Target/TargetMachine.h"
finalizeOptimizationRemarks(std::unique_ptr<tool_output_file> DiagOutputFile) {
   // Make sure we flush the diagnostic remarks file in case the linker doesn't
   // call the global destructors before exiting.
   if (!DiagOutputFile)
     return;
   DiagOutputFile->keep();
   DiagOutputFile->os().flush();
 }
 
-static void handleAsmUndefinedRefs(Module &Mod, TargetMachine &TM) {
-  // Collect the list of undefined symbols used in asm and update
-  // llvm.compiler.used to prevent optimization to drop these from the output.
-  StringSet<> AsmUndefinedRefs;
-  ModuleSymbolTable::CollectAsmSymbols(
-      Mod,
-      [&AsmUndefinedRefs](StringRef Name, object::BasicSymbolRef::Flags Flags) {
-        if (Flags & object::BasicSymbolRef::SF_Undefined)
-          AsmUndefinedRefs.insert(Name);
-      });
-  updateCompilerUsed(Mod, TM, AsmUndefinedRefs);
-}
-
 Error lto::backend(Config &C, AddStreamFn AddStream,
                    unsigned ParallelCodeGenParallelismLevel,
                    std::unique_ptr<Module> Mod,
                    ModuleSummaryIndex &CombinedIndex) {
   Expected<const Target *> TOrErr = initAndLookupTarget(C, *Mod);
   if (!TOrErr)
     return TOrErr.takeError();
 
   std::unique_ptr<TargetMachine> TM =
       createTargetMachine(C, Mod->getTargetTriple(), *TOrErr);
 
-  handleAsmUndefinedRefs(*Mod, *TM);
-
   // Setup optimization remarks.
   auto DiagFileOrErr = lto::setupOptimizationRemarks(
       Mod->getContext(), C.RemarksFilename, C.RemarksWithHotness);
   if (!DiagFileOrErr)
     return DiagFileOrErr.takeError();
   auto DiagnosticOutputFile = std::move(*DiagFileOrErr);
 
   if (!C.CodeGenOnly) {
Error lto::thinBackend(Config &Conf, unsigned Task, AddStreamFn AddStream,
                        MapVector<StringRef, BitcodeModule> &ModuleMap) {
   Expected<const Target *> TOrErr = initAndLookupTarget(Conf, Mod);
   if (!TOrErr)
     return TOrErr.takeError();
 
   std::unique_ptr<TargetMachine> TM =
       createTargetMachine(Conf, Mod.getTargetTriple(), *TOrErr);
 
-  handleAsmUndefinedRefs(Mod, *TM);
-
   if (Conf.CodeGenOnly) {
     codegen(Conf, TM.get(), AddStream, Task, Mod);
     return Error::success();
   }
 
   if (Conf.PreOptModuleHook && !Conf.PreOptModuleHook(Task, Mod))
     return Error::success();
 

llvm/trunk/lib/Object/IRSymtab.cpp

Error Builder::addSymbol(const ModuleSymbolTable &Msymtab,
   if (Flags & object::BasicSymbolRef::SF_FormatSpecific)
     Sym.Flags |= 1 << storage::Symbol::FB_format_specific;
   if (Flags & object::BasicSymbolRef::SF_Executable)
     Sym.Flags |= 1 << storage::Symbol::FB_executable;
 
   Sym.ComdatIndex = -1;
   auto *GV = Msym.dyn_cast<GlobalValue *>();
   if (!GV) {
+    // Undefined module asm symbols act as GC roots and are implicitly used.
+    if (Flags & object::BasicSymbolRef::SF_Undefined)
+      Sym.Flags |= 1 << storage::Symbol::FB_used;
     setStr(Sym.IRName, "");
     return Error::success();
   }
 
   setStr(Sym.IRName, GV->getName());
 
   if (Used.count(GV))
     Sym.Flags |= 1 << storage::Symbol::FB_used;

llvm/trunk/test/LTO/Resolution/X86/Inputs/mod-asm-used.ll

+target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+@foo = global i32 1, align 4

llvm/trunk/test/LTO/Resolution/X86/mod-asm-used.ll

+; RUN: opt -module-summary -o %t.bc %s
+; RUN: opt -module-summary -o %t2.bc %S/Inputs/mod-asm-used.ll
+; RUN: llvm-lto2 run %t.bc -r %t.bc,foo,l %t2.bc -r %t2.bc,foo,pl -o %t3
+; RUN: llvm-nm %t3.1 | FileCheck %s
+
+target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+; CHECK: D foo
+module asm ".quad foo"

llvm/trunk/test/tools/gold/X86/asm_undefined2.ll

 ; RegularLTO testcase
 ; RUN: llvm-as %s -o %t.o
 ; RUN: %gold -shared -m elf_x86_64 -o %t2 -plugin %llvmshlibdir/LLVMgold.so \
 ; RUN: %t.o --plugin-opt=save-temps -upatatino
 ; RUN: llvm-dis < %t2.0.5.precodegen.bc | FileCheck %s
 
 ; ThinLTO testcase
 ; RUN: opt -module-summary %s -o %t.o
 ; RUN: %gold -m elf_x86_64 -plugin %llvmshlibdir/LLVMgold.so \
 ; RUN:     --plugin-opt=save-temps \
 ; RUN:     --plugin-opt=thinlto -o %t2 %t.o
 ; RUN: llvm-dis < %t.o.5.precodegen.bc | FileCheck %s
 
-; Check that foo is properly appended to llvm.compiler.used
-; CHECK: @llvm.compiler.used = appending global [1 x i8*] [i8* bitcast (void ()* @foo to i8*)], section "llvm.metadata"
+; Check that foo is not internalized
+; CHECK: define void @foo
 
 target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
 target triple = "x86_64-unknown-linux-gnu"
 
 module asm ".global patatino"
 module asm ".equ patatino, foo"
 
 declare void @patatino()
 
 define void @foo() {
   call void @patatino()
   ret void
 }