[LTO] Add module asm uses to the llvm.compiler.used before merging

[tejohnson]

Summary

In LTO mode we were adding undefined module asm uses to the
llvm.compiler.used eventually, but not early enough. The referenced
internal symbol was simply not being linked into the combined module
since it didn't appear referenced.

Fixes PR25679.

Depends on D30585

[pcc]

Will your test case work without LTO? I'd expect the user to have to use __attribute((used)) to prevent the io_cancel_local_0_4 symbol from being DCE'd.

[mehdi_amini]

test/LTO/X86/symver-asm.ll

2

The comment in the test below says " Even without -exported-symbol, io_cancel_0_4 should be noticed by LTOModule's" ?

[tejohnson]

In D30588#693713, @pcc wrote:

Will your test case work without LTO? I'd expect the user to have to use __attribute((used)) to prevent the io_cancel_local_0_4 symbol from being DCE'd.

I tried it with "opt -O2" and indeed the local symbol is removed. However, there is existing handling in the LTO path for this case of having uses in the module asm that are not (yet) on the llvm.compiler.used. As noted we are already adding these referenced symbols to the llvm.compiler.used in both the old and new LTO APIs, just later on. I looked back at the history of this. The handling in the new LTO API was added with D24617. This would have made the new LTO API consistent with the old LTO API. Chasing through the history of where this was added in libLTO, I found this old commit that added it. Note the comment "The second improvement..." which refers to this. So at least on the LTO path, there doesn't seem to be an expectation that these were already added to the llvm.compiler.used.

commit 38c4e535493363b96eac47af9e7c056530137bea
Author: Rafael Espindola <[e-mail]>
Date: Wed Mar 2 04:14:42 2011 +0000

Add a special streamer to libLTO that just records symbols definitions and
uses.

The result produced by the streamer is used to give the linker more accurate
information and to add to llvm.compiler.used. The second improvement removes
the need for the user to add __attribute__((used)) to functions only used in
inline asm. The first one lets us build firefox with LTO on Darwin :-)

git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@126830 91177308-0d34-0410-b5e6-96231b3b80d8

test/LTO/X86/symver-asm.ll

2

It is noticed and kept, but internalized. I wanted to ensure that we get the desired global binding for the versioned alias, so I had to make it exported here. Notice that before we were only checking for the presence of the io_cancel* symbols in the nm output, not looking at their symbol binding. I will remove the comment below which is now stale. (Also the second -exported-symbol above crept in from another version of the test, I will remove it)

[tejohnson]

Update test per review.

[mehdi_amini]

*internal* functions referenced from ASM need to be in llvm.compiler.used, regular global functions don't.

[mehdi_amini]

(IIUC).

[tejohnson]

In D30588#694472, @mehdi_amini wrote:

*internal* functions referenced from ASM need to be in llvm.compiler.used, regular global functions don't.

Confirmed - I had forgotten that distinction. This change is no longer required then. I will update the follow-on fixes to include the llvm.compiler.used for the local.

include/llvm/LTO/LTOBackend.h

Error backend(Config &C, AddStreamFn AddStream,
               std::unique_ptr<Module> M, ModuleSummaryIndex &CombinedIndex);
 
 /// Runs a ThinLTO backend.
 Error thinBackend(Config &C, unsigned Task, AddStreamFn AddStream, Module &M,
                   ModuleSummaryIndex &CombinedIndex,
                   const FunctionImporter::ImportMapTy &ImportList,
                   const GVSummaryMapTy &DefinedGlobals,
                   MapVector<StringRef, BitcodeModule> &ModuleMap);
+
+/// Collect the list of undefined symbols used in asm in the Module and update
+/// llvm.compiler.used to prevent optimization from dropping these from the IR.
+Error handleAsmUndefinedRefsInMod(Config &C, Module &Mod);
 }
 }
 
 #endif

include/llvm/LTO/legacy/LTOCodeGenerator.h

struct LTOCodeGenerator {
   /// Resets \a HasVerifiedInput.
   bool addModule(struct LTOModule *);
 
   /// Set the destination module.
   ///
   /// Resets \a HasVerifiedInput.
   void setModule(std::unique_ptr<LTOModule> M);
 
-  void setAsmUndefinedRefs(struct LTOModule *);
   void setTargetOptions(const TargetOptions &Options);
   void setDebugInfo(lto_debug_model);
   void setCodePICModel(Optional<Reloc::Model> Model) { RelocModel = Model; }
 
   /// Set the file type to be emitted (assembly or object code).
   /// The default is TargetMachine::CGFT_ObjectFile.
   void setFileType(TargetMachine::CodeGenFileType FT) { FileType = FT; }
 
private:
   std::unique_ptr<Module> MergedModule;
   std::unique_ptr<Linker> TheLinker;
   std::unique_ptr<TargetMachine> TargetMach;
   bool EmitDwarfDebugInfo = false;
   bool ScopeRestrictionsDone = false;
   bool HasVerifiedInput = false;
   Optional<Reloc::Model> RelocModel;
   StringSet<> MustPreserveSymbols;
-  StringSet<> AsmUndefinedRefs;
   StringMap<GlobalValue::LinkageTypes> ExternalSymbols;
   std::vector<std::string> CodegenOptions;
   std::string FeatureStr;
   std::string MCpu;
   std::string MAttr;
   std::string NativeObjectPath;
   TargetOptions Options;
   CodeGenOpt::Level CGOptLevel = CodeGenOpt::Default;

include/llvm/LTO/legacy/LTOModule.h

public:
   const GlobalValue *getSymbolGV(uint32_t index) {
     if (index < _symbols.size())
       return _symbols[index].symbol;
     return nullptr;
   }
 
   StringRef getLinkerOpts() { return LinkerOpts; }
 
-  const std::vector<StringRef> &getAsmUndefinedRefs() { return _asm_undefines; }
-
 private:
   /// Parse metadata from the module
   // FIXME: it only parses "Linker Options" metadata at the moment
   void parseMetadata();
 
   /// Parse the symbols from the module and model-level ASM and add them to
   /// either the defined or undefined lists.
   void parseSymbols();
 
+  /// Update the llvm.compiler_used globals to force preserving libcalls and
+  /// symbols referenced from asm.
+  void updateCompilerUsed();
+
   /// Add a symbol which isn't defined just yet to a list to be resolved later.
   void addPotentialUndefinedSymbol(ModuleSymbolTable::Symbol Sym,
                                    bool isFunc);
 
   /// Add a defined symbol to the list.
   void addDefinedSymbol(StringRef Name, const GlobalValue *def,
                         bool isFunction);
 

lib/LTO/LTO.cpp

Error LTO::addRegularLTO(BitcodeModule BM, const SymbolResolution *&ResI,
   Module &M = **MOrErr;
   if (Error Err = M.materializeMetadata())
     return Err;
   UpgradeDebugInfo(M);
 
   ModuleSymbolTable SymTab;
   SymTab.addModule(&M);
 
+  // Update the llvm.compiler.used with symbols referenced from module
+  // level asm so that we keep the necessary symbols when merging into
+  // the combined module.
+  if (Error Err = handleAsmUndefinedRefsInMod(Conf, M))
+    return Err;
   SmallPtrSet<GlobalValue *, 8> Used;
   collectUsedGlobalVariables(M, Used, /*CompilerUsed*/ false);
 
   std::vector<GlobalValue *> Keep;
 
   for (GlobalVariable &GV : M.globals())
     if (GV.hasAppendingLinkage())
       Keep.push_back(&GV);

lib/LTO/LTOBackend.cpp

ModuleSymbolTable::CollectAsmSymbols(
       Triple(Mod.getTargetTriple()), Mod.getModuleInlineAsm(),
       [&AsmUndefinedRefs](StringRef Name, object::BasicSymbolRef::Flags Flags) {
         if (Flags & object::BasicSymbolRef::SF_Undefined)
           AsmUndefinedRefs.insert(Name);
       });
   updateCompilerUsed(Mod, TM, AsmUndefinedRefs);
 }
 
+Error lto::handleAsmUndefinedRefsInMod(Config &C, Module &Mod) {
+  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);
+  return Error::success();
+}
+
 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) {

lib/LTO/LTOCodeGenerator.cpp

 #include "llvm/IR/LLVMContext.h"
 #include "llvm/IR/LegacyPassManager.h"
 #include "llvm/IR/Mangler.h"
 #include "llvm/IR/Module.h"
 #include "llvm/IR/Verifier.h"
 #include "llvm/InitializePasses.h"
 #include "llvm/LTO/LTO.h"
 #include "llvm/LTO/legacy/LTOModule.h"
-#include "llvm/LTO/legacy/UpdateCompilerUsed.h"
 #include "llvm/Linker/Linker.h"
 #include "llvm/MC/MCAsmInfo.h"
 #include "llvm/MC/MCContext.h"
 #include "llvm/MC/SubtargetFeature.h"
 #include "llvm/Support/CommandLine.h"
 #include "llvm/Support/FileSystem.h"
 #include "llvm/Support/Host.h"
 #include "llvm/Support/MemoryBuffer.h"
void LTOCodeGenerator::initializeLTOPasses() {
   initializeLegacyLICMPassPass(R);
   initializeMergedLoadStoreMotionLegacyPassPass(R);
   initializeGVNLegacyPassPass(R);
   initializeMemCpyOptLegacyPassPass(R);
   initializeDCELegacyPassPass(R);
   initializeCFGSimplifyPassPass(R);
 }
 
-void LTOCodeGenerator::setAsmUndefinedRefs(LTOModule *Mod) {
-  const std::vector<StringRef> &undefs = Mod->getAsmUndefinedRefs();
-  for (int i = 0, e = undefs.size(); i != e; ++i)
-    AsmUndefinedRefs[undefs[i]] = 1;
-}
-
 bool LTOCodeGenerator::addModule(LTOModule *Mod) {
   assert(&Mod->getModule().getContext() == &Context &&
          "Expected module in same context");
 
   bool ret = TheLinker->linkInModule(Mod->takeModule());
-  setAsmUndefinedRefs(Mod);
 
   // We've just changed the input, so let's make sure we verify it.
   HasVerifiedInput = false;
 
   return !ret;
 }
 
 void LTOCodeGenerator::setModule(std::unique_ptr<LTOModule> Mod) {
   assert(&Mod->getModule().getContext() == &Context &&
          "Expected module in same context");
 
-  AsmUndefinedRefs.clear();
-
   MergedModule = Mod->takeModule();
   TheLinker = make_unique<Linker>(*MergedModule);
-  setAsmUndefinedRefs(&*Mod);
 
   // We've just changed the input, so let's make sure we verify it.
   HasVerifiedInput = false;
 }
 
 void LTOCodeGenerator::setTargetOptions(const TargetOptions &Options) {
   this->Options = Options;
 }
if (ShouldRestoreGlobalsLinkage) {
     for (auto &GV : *MergedModule)
       RecordLinkage(GV);
     for (auto &GV : MergedModule->globals())
       RecordLinkage(GV);
     for (auto &GV : MergedModule->aliases())
       RecordLinkage(GV);
   }
 
-  // Update the llvm.compiler_used globals to force preserving libcalls and
-  // symbols referenced from asm
-  updateCompilerUsed(*MergedModule, *TargetMach, AsmUndefinedRefs);
-
   internalizeModule(*MergedModule, mustPreserveGV);
 
   ScopeRestrictionsDone = true;
 }
 
 /// Restore original linkage for symbols that may have been internalized
 void LTOCodeGenerator::restoreLinkageForExternals() {
   if (!ShouldInternalize || !ShouldRestoreGlobalsLinkage)

lib/LTO/LTOModule.cpp

 #include "llvm/Bitcode/BitcodeReader.h"
 #include "llvm/CodeGen/Analysis.h"
 #include "llvm/CodeGen/TargetLoweringObjectFileImpl.h"
 #include "llvm/IR/Constants.h"
 #include "llvm/IR/DiagnosticPrinter.h"
 #include "llvm/IR/LLVMContext.h"
 #include "llvm/IR/Metadata.h"
 #include "llvm/IR/Module.h"
+#include "llvm/LTO/legacy/UpdateCompilerUsed.h"
 #include "llvm/MC/MCExpr.h"
 #include "llvm/MC/MCInst.h"
 #include "llvm/MC/MCInstrInfo.h"
 #include "llvm/MC/MCParser/MCAsmParser.h"
 #include "llvm/MC/MCParser/MCTargetAsmParser.h"
 #include "llvm/MC/MCSection.h"
 #include "llvm/MC/MCSubtargetInfo.h"
 #include "llvm/MC/MCSymbol.h"
LTOModule::makeLTOModule(MemoryBufferRef Buffer, const TargetOptions &options,
   }
 
   TargetMachine *target =
       march->createTargetMachine(TripleStr, CPU, FeatureStr, options, None);
 
   std::unique_ptr<LTOModule> Ret(new LTOModule(std::move(M), Buffer, target));
   Ret->parseSymbols();
   Ret->parseMetadata();
+  // Update the llvm.compiler_used globals to force preserving libcalls and
+  // symbols referenced from asm, to ensure the necessary symbols are
+  // kept when merging the module.
+  Ret->updateCompilerUsed();
 
   return std::move(Ret);
 }
 
 /// Create a MemoryBuffer from a memory range with an optional name.
 std::unique_ptr<MemoryBuffer>
 LTOModule::makeBuffer(const void *mem, size_t length, StringRef name) {
   const char *startPtr = (const char*)mem;
for (StringMap<NameAndAttributes>::iterator u =_undefines.begin(),
     // If this symbol also has a definition, then don't make an undefine because
     // it is a tentative definition.
     if (_defines.count(u->getKey())) continue;
     NameAndAttributes info = u->getValue();
     _symbols.push_back(info);
   }
 }
 
+void LTOModule::updateCompilerUsed() {
+  StringSet<> AsmUndefinedRefs;
+  for (int i = 0, e = _asm_undefines.size(); i != e; ++i)
+    AsmUndefinedRefs[_asm_undefines[i]] = 1;
+  llvm::updateCompilerUsed(getModule(), *_target, AsmUndefinedRefs);
+}
+
 /// parseMetadata - Parse metadata from the module
 void LTOModule::parseMetadata() {
   raw_string_ostream OS(LinkerOpts);
 
   // Linker Options
   if (Metadata *Val = getModule().getModuleFlag("Linker Options")) {
     MDNode *LinkerOptions = cast<MDNode>(Val);
     for (unsigned i = 0, e = LinkerOptions->getNumOperands(); i != e; ++i) {

test/LTO/X86/symver-asm.ll

 ; RUN: llvm-as < %s >%t1
-; RUN: llvm-lto -o %t2 %t1
+; RUN: llvm-lto -exported-symbol=io_cancel_0_4 -o %t2 %t1
 ; RUN: llvm-nm %t2 | FileCheck %s
+; RUN: llvm-lto2 -r %t1,io_cancel_0_4,plx -r %t1,io_cancel_0_4,plx -r %t1,io_cancel_local_0_4,plx -o %t3 %t1 -save-temps
+; RUN: llvm-nm %t3.0 | FileCheck %s
+; RUN: llvm-dis %t3.0.2.internalize.bc -o - | FileCheck %s --check-prefix=INTERN
 
+target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
 target triple = "x86_64-unknown-linux-gnu"
 
 module asm ".symver io_cancel_0_4,io_cancel@@LIBAIO_0.4"
+module asm ".symver io_cancel_local_0_4,io_cancel_local@@LIBAIO_0.4"
 
-; Even without -exported-symbol, io_cancel_0_4 should be noticed by LTOModule's
-; RecordStreamer, so it shouldn't get eliminated. However, the object file will
-; contain the aliased symver as well as the original.
 define i32 @io_cancel_0_4() {
-; CHECK: io_cancel@@LIBAIO_0.4
-; CHECK: io_cancel_0_4
+; CHECK-DAG: T io_cancel@@LIBAIO_0.4
+; CHECK-DAG: T io_cancel_0_4
+  ret i32 0
+}
+
+define internal i32 @io_cancel_local_0_4() {
+; INTERN: llvm.compiler.used {{.*}} @io_cancel_local_0_4
+; INTERN: define internal i32 @io_cancel_local_0_4()
+; CHECK-DAG: t io_cancel_local@@LIBAIO_0.4
+; CHECK-DAG: t io_cancel_local_0_4
   ret i32 0
 }

test/tools/llvm-lto2/errors.ll

 ; RUN: llvm-as %s -o %t.bc
 ; RUN: not llvm-lto2 -o %t2.o %t.bc 2>&1 | FileCheck --check-prefix=ERR1 %s
 ; RUN: not llvm-lto2 -o %t2.o -r %t.bc,foo,p -r %t.bc,bar,p %t.bc 2>&1 | FileCheck --check-prefix=ERR2 %s
 ; RUN: not llvm-lto2 -o %t2.o -r %t.bc,foo,q %t.bc 2>&1 | FileCheck --check-prefix=ERR3 %s
 ; RUN: not llvm-lto2 -o %t2.o -r foo %t.bc 2>&1 | FileCheck --check-prefix=ERR4 %s
 
 ; ERR1: missing symbol resolution for {{.*}}.bc,foo
 ; ERR2: unused symbol resolution for {{.*}}.bc,bar
 ; ERR3: invalid character q in resolution: {{.*}}.bc,foo
 ; ERR4: invalid resolution: foo
 
 target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
 
 @foo = global i32 0

tools/llvm-lto2/llvm-lto2.cpp

for (std::string R : SymbolResolutions) {
     SymbolResolution Res;
     for (char C : Rest) {
       if (C == 'p')
         Res.Prevailing = true;
       else if (C == 'l')
         Res.FinalDefinitionInLinkageUnit = true;
       else if (C == 'x')
         Res.VisibleToRegularObj = true;
-      else
+      else {
         llvm::errs() << "invalid character " << C << " in resolution: " << R
                      << '\n';
+        return 1;
+      }
     }
     CommandLineResolutions[{FileName, SymbolName}].push_back(Res);
   }
 
   std::vector<std::unique_ptr<MemoryBuffer>> MBs;
 
   Config Conf;
   Conf.DiagHandler = [](const DiagnosticInfo &DI) {