[LTO] Asm undefs should be included in llvm.compiler_used

[davide]

Summary

This fixes PR28218. For further reference, and my analysis, please take a look at:
https://llvm.org/bugs/show_bug.cgi?id=28218

[ruiu]

ELF/LTO.cpp

209–220

This function is already too long. Can you separate this code as a new function?

220

AsmUndefinedRefs.insert(Name) since it is a set.

[davide]

Rui's comments.

[ruiu]

ELF/LTO.cpp

158

remove llvm::.

169

I think you don't need this brace to create a scope. raw_svector_ostream is unbuffered so Buffer is always up-to-date.

173

Why not Buffer.str()? I'd rename Buffer Name and just call AsmUndefinedRefs.insert(Name.str())

[davide]

Rui's comments (2).

[ruiu]

Looks good, but I guess you want someone else to work on LTO to sign off.

[• rafael]

LGTM with one last pass of reviews.

ELF/LTO.cpp

28

Please make sure this builds with -DBUILD_SHARED_LIBS=ON.

159

s/no asm/not an assembly symbol/ I guess?

163

While are the comment lines so short?

I think you can drop the "referenced": This is an undefined reference to a symbol in asm".

test/ELF/lto/asmundef.ll

23

Check that foo is internalized.

ELF/LTO.h

 private:
   std::vector<std::unique_ptr<InputFile>> runSplitCodegen(
       const std::function<std::unique_ptr<llvm::TargetMachine>()> &TMFactory);
 
   std::unique_ptr<llvm::Module> Combined;
   llvm::IRMover Mover;
   std::vector<SmallString<0>> OwningData;
   llvm::StringSet<> InternalizedSyms;
+  llvm::StringSet<> AsmUndefinedRefs;
   std::string TheTriple;
 };
 }
 }
 
 #endif

ELF/LTO.cpp

 #include "llvm/Analysis/TargetTransformInfo.h"
 #include "llvm/Bitcode/ReaderWriter.h"
 #include "llvm/CodeGen/CommandFlags.h"
 #include "llvm/CodeGen/ParallelCG.h"
 #include "llvm/IR/AutoUpgrade.h"
 #include "llvm/IR/LegacyPassManager.h"
 #include "llvm/IR/PassManager.h"
 #include "llvm/IR/Verifier.h"
+#include "llvm/LTO/UpdateCompilerUsed.h"
 #include "llvm/Linker/IRMover.h"
 #include "llvm/Passes/PassBuilder.h"
 #include "llvm/Support/StringSaver.h"
 #include "llvm/Support/TargetRegistry.h"
 #include "llvm/Target/TargetMachine.h"
 #include "llvm/Transforms/IPO.h"
 #include "llvm/Transforms/IPO/PassManagerBuilder.h"
 #include "llvm/Transforms/Utils/ModuleUtils.h"

 BitcodeCompiler::BitcodeCompiler()
     : Combined(new llvm::Module("ld-temp.o", Driver->Context)),
       Mover(*Combined) {}
 
 static void undefine(Symbol *S) {
   replaceBody<Undefined>(S, S->body()->getName(), STV_DEFAULT, S->body()->Type);
 }
 
+static void handleUndefinedAsmRefs(const BasicSymbolRef &Sym, GlobalValue *GV,
+                                   StringSet<> &AsmUndefinedRefs) {
+  // GV associated => no asm, bail out.
+  if (GV)
+    return;
+
+  // This is an undefined reference to a symbol
+  // referenced from asm. We put that in compiler.used,
+  // so that we can preserve it from being dropped from
+  // the output, without necessarily preventing its
+  // internalization.
+  SmallString<64> Name;
+  raw_svector_ostream OS(Name);
+  Sym.printName(OS);
+  AsmUndefinedRefs.insert(Name.str());
+}
+
 void BitcodeCompiler::add(BitcodeFile &F) {
   std::unique_ptr<IRObjectFile> Obj = std::move(F.Obj);
   std::vector<GlobalValue *> Keep;
   unsigned BodyIndex = 0;
   ArrayRef<Symbol *> Syms = F.getSymbols();
 
   Module &M = Obj->getModule();
   if (M.getDataLayoutStr().empty())
void BitcodeCompiler::add(BitcodeFile &F) {
   for (const BasicSymbolRef &Sym : Obj->symbols()) {
     uint32_t Flags = Sym.getFlags();
     GlobalValue *GV = Obj->getSymbolGV(Sym.getRawDataRefImpl());
     if (GV && GV->hasAppendingLinkage())
       Keep.push_back(GV);
     if (BitcodeFile::shouldSkip(Flags))
       continue;
     Symbol *S = Syms[BodyIndex++];
-    if (Flags & BasicSymbolRef::SF_Undefined)
+    if (Flags & BasicSymbolRef::SF_Undefined) {
+      handleUndefinedAsmRefs(Sym, GV, AsmUndefinedRefs);
       continue;
+    }
     auto *B = dyn_cast<DefinedBitcode>(S->body());
     if (!B || B->File != &F)
       continue;
 
     // We collect the set of symbols we want to internalize here
     // and change the linkage after the IRMover executed, i.e. after
     // we imported the symbols and satisfied undefined references
     // to it. We can't just change linkage here because otherwise
     // the IRMover will just rename the symbol.
     if (GV && shouldInternalize(Used, S, GV))
       InternalizedSyms.insert(GV->getName());
 
     // At this point we know that either the combined LTO object will provide a
     // definition of a symbol, or we will internalize it. In either case, we
     // need to undefine the symbol. In the former case, the real definition
     // needs to be able to replace the original definition without conflicting.
     // In the latter case, we need to allow the combined LTO object to provide a
     // definition with the same name, for example when doing parallel codegen.
     undefine(S);
 
     if (!GV)
       // Module asm symbol.
       continue;

 std::vector<std::unique_ptr<InputFile>> BitcodeCompiler::compile() {
   TheTriple = Combined->getTargetTriple();
   for (const auto &Name : InternalizedSyms) {
     GlobalValue *GV = Combined->getNamedValue(Name.first());
     assert(GV);
     internalize(*GV);
   }
 
-  if (Config->SaveTemps)
-    saveBCFile(*Combined, ".lto.bc");
-
   std::string Msg;
   const Target *T = TargetRegistry::lookupTarget(TheTriple, Msg);
   if (!T)
     fatal("target not found: " + Msg);
   TargetOptions Options = InitTargetOptionsFromCodeGenFlags();
   Reloc::Model R = Config->Pic ? Reloc::PIC_ : Reloc::Static;
 
   auto CreateTargetMachine = [&]() {
     return std::unique_ptr<TargetMachine>(
         T->createTargetMachine(TheTriple, "", "", Options, R));
   };
 
   std::unique_ptr<TargetMachine> TM = CreateTargetMachine();
+
+  // Update llvm.compiler.used so that optimizations won't strip
+  // off AsmUndefinedReferences.
+  UpdateCompilerUsed(*Combined, *TM, AsmUndefinedRefs);
+
+  if (Config->SaveTemps)
+    saveBCFile(*Combined, ".lto.bc");
+
   runLTOPasses(*Combined, *TM);
   if (HasError)
     return {};
 
   return runSplitCodegen(CreateTargetMachine);
 }

test/ELF/lto/asmundef.ll

+; REQUIRES: x86
+; RUN: llvm-as %s -o %t.o
+; RUN: ld.lld -m elf_x86_64 %t.o -o %t -save-temps
+; RUN: llvm-dis %t.lto.bc -o - | FileCheck %s
+
+target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+module asm ".weak patatino"
+module asm ".equ patatino, foo"
+
+declare void @patatino()
+
+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"