Index: include/llvm/IR/ModuleSummaryIndex.h =================================================================== --- include/llvm/IR/ModuleSummaryIndex.h +++ include/llvm/IR/ModuleSummaryIndex.h @@ -192,6 +192,9 @@ /// Return true if this global value is located in a specific section. bool hasSection() const { return Flags.HasSection; } + /// Flag that this global value is located in a specific section. + void setHasSection() { Flags.HasSection = true; } + /// Record a reference from this global value to the global value identified /// by \p RefGUID. void addRefEdge(GlobalValue::GUID RefGUID) { RefEdgeList.push_back(RefGUID); } @@ -414,17 +417,19 @@ /// Returns the first GlobalValueSummary for \p GV, asserting that there /// is only one if \p PerModuleIndex. GlobalValueSummary *getGlobalValueSummary(const GlobalValue &GV, - bool PerModuleIndex = true) const { + bool PerModuleIndex = true, + bool AssertExists = true) const { assert(GV.hasName() && "Can't get GlobalValueSummary for GV with no name"); return getGlobalValueSummary(GlobalValue::getGUID(GV.getName()), - PerModuleIndex); + PerModuleIndex, AssertExists); } /// Returns the first GlobalValueSummary for \p ValueGUID, asserting that /// there /// is only one if \p PerModuleIndex. GlobalValueSummary *getGlobalValueSummary(GlobalValue::GUID ValueGUID, - bool PerModuleIndex = true) const; + bool PerModuleIndex = true, + bool AssertExists = true) const; /// Table of modules, containing module hash and id. const StringMap> &modulePaths() const { Index: include/llvm/Support/TargetRegistry.h =================================================================== --- include/llvm/Support/TargetRegistry.h +++ include/llvm/Support/TargetRegistry.h @@ -280,6 +280,9 @@ /// hasMCAsmBackend - Check if this target supports .o generation. bool hasMCAsmBackend() const { return MCAsmBackendCtorFn != nullptr; } + /// hasMCAsmParser - Check if this target supports assembly parsing. + bool hasMCAsmParser() const { return MCAsmParserCtorFn != nullptr; } + /// @} /// @name Feature Constructors /// @{ Index: include/llvm/Transforms/Utils/FunctionImportUtils.h =================================================================== --- include/llvm/Transforms/Utils/FunctionImportUtils.h +++ include/llvm/Transforms/Utils/FunctionImportUtils.h @@ -55,7 +55,7 @@ /// Get the name for SGV that should be used in the linked destination /// module. Specifically, this handles the case where we need to rename /// a local that is being promoted to global scope. - std::string getName(const GlobalValue *SGV); + std::string getName(const GlobalValue *SGV, bool DoPromote); /// Process globals so that they can be used in ThinLTO. This includes /// promoting local variables so that they can be reference externally by @@ -67,7 +67,7 @@ /// Get the new linkage for SGV that should be used in the linked destination /// module. Specifically, for ThinLTO importing or exporting it may need /// to be adjusted. - GlobalValue::LinkageTypes getLinkage(const GlobalValue *SGV); + GlobalValue::LinkageTypes getLinkage(const GlobalValue *SGV, bool DoPromote); public: FunctionImportGlobalProcessing( Index: lib/Analysis/LLVMBuild.txt =================================================================== --- lib/Analysis/LLVMBuild.txt +++ lib/Analysis/LLVMBuild.txt @@ -19,4 +19,4 @@ type = Library name = Analysis parent = Libraries -required_libraries = Core Support ProfileData +required_libraries = Core Support ProfileData Object Index: lib/Analysis/ModuleSummaryAnalysis.cpp =================================================================== --- lib/Analysis/ModuleSummaryAnalysis.cpp +++ lib/Analysis/ModuleSummaryAnalysis.cpp @@ -13,6 +13,7 @@ //===----------------------------------------------------------------------===// #include "llvm/Analysis/ModuleSummaryAnalysis.h" +#include "llvm/ADT/Triple.h" #include "llvm/Analysis/BlockFrequencyInfo.h" #include "llvm/Analysis/BlockFrequencyInfoImpl.h" #include "llvm/Analysis/BranchProbabilityInfo.h" @@ -24,6 +25,7 @@ #include "llvm/IR/InstIterator.h" #include "llvm/IR/IntrinsicInst.h" #include "llvm/IR/ValueSymbolTable.h" +#include "llvm/Object/IRObjectFile.h" #include "llvm/Pass.h" using namespace llvm; @@ -77,7 +79,9 @@ static void computeFunctionSummary(ModuleSummaryIndex &Index, const Module &M, const Function &F, BlockFrequencyInfo *BFI, - ProfileSummaryInfo *PSI) { + ProfileSummaryInfo *PSI, + SmallPtrSetImpl &LocalsUsed, + bool &HasInlineAsm) { // Summary not currently supported for anonymous functions, they must // be renamed. if (!F.hasName()) @@ -90,6 +94,7 @@ DenseMap IndirectCallEdges; DenseSet RefEdges; ICallPromotionAnalysis ICallAnalysis; + bool HasLocalsInUsed = !LocalsUsed.empty(); SmallPtrSet Visited; for (const BasicBlock &BB : F) @@ -101,6 +106,13 @@ auto CS = ImmutableCallSite(&I); if (!CS) continue; + + const auto *CI = dyn_cast(&I); + if (HasLocalsInUsed && CI && CI->isInlineAsm()) { + RefEdges.insert(LocalsUsed.begin(), LocalsUsed.end()); + HasInlineAsm = true; + } + auto *CalledValue = CS.getCalledValue(); auto *CalledFunction = CS.getCalledFunction(); // Check if this is an alias to a function. If so, get the @@ -126,7 +138,6 @@ : CalleeInfo::HotnessType::Unknown; CallGraphEdges[CalleeId].updateHotness(Hotness); } else { - const auto *CI = dyn_cast(&I); // Skip inline assembly calls. if (CI && CI->isInlineAsm()) continue; @@ -166,16 +177,35 @@ Index.addGlobalValueSummary(V.getName(), std::move(GVarSummary)); } +static void computeAliasSummary(ModuleSummaryIndex &Index, + const GlobalAlias &A) { + GlobalValueSummary::GVFlags Flags(A); + std::unique_ptr AS = llvm::make_unique(Flags); + auto *Aliasee = A.getBaseObject(); + if (!Aliasee->hasName()) + // Nameless function don't have an entry in the summary, skip + // it. + return; + auto *AliaseeSummary = Index.getGlobalValueSummary(*Aliasee); + assert(AliaseeSummary && "Alias expects aliasee summary to be parsed"); + AS->setAliasee(AliaseeSummary); + Index.addGlobalValueSummary(A.getName(), std::move(AS)); +} + ModuleSummaryIndex llvm::buildModuleSummaryIndex( const Module &M, std::function GetBFICallback, ProfileSummaryInfo *PSI) { ModuleSummaryIndex Index; - // Check if the module can be promoted, otherwise just disable importing from - // it by not emitting any summary. - // FIXME: we could still import *into* it most of the time. - if (!moduleCanBeRenamedForThinLTO(M)) - return Index; + SmallPtrSet Used; + collectUsedGlobalVariables(M, Used, /*CompilerUsed*/ false); + SmallPtrSet LocalsUsed; + for (auto *V : Used) { + if (V->hasLocalLinkage()) + LocalsUsed.insert(V); + } + + bool HasInlineAsm = false; // Compute summaries for all functions defined in module, and save in the // index. @@ -194,7 +224,7 @@ BFI = BFIPtr.get(); } - computeFunctionSummary(Index, M, F, BFI, PSI); + computeFunctionSummary(Index, M, F, BFI, PSI, LocalsUsed, HasInlineAsm); } // Compute summaries for all variables defined in module, and save in the @@ -204,6 +234,57 @@ continue; computeVariableSummary(Index, G); } + + // Compute summaries for all aliases defined in module, and save in the + // index. + for (const GlobalAlias &A : M.aliases()) + computeAliasSummary(Index, A); + + if (HasInlineAsm) { + for (auto *V : LocalsUsed) { + auto *Summary = Index.getGlobalValueSummary(*V); + assert(Summary && "Missing summary for global value"); + Summary->setHasSection(); + } + } + + if (!M.getModuleInlineAsm().empty()) { + object::IRObjectFile::CollectAsmUndefinedRefs( + Triple(M.getTargetTriple()), M.getModuleInlineAsm(), + [&M, &Index](StringRef Name, object::BasicSymbolRef::Flags Flags) { + if (!(Flags & (object::BasicSymbolRef::SF_Weak || + object::BasicSymbolRef::SF_Global))) { + GlobalValue *GV = M.getNamedValue(Name); + if (!GV) + return; + if (dyn_cast(GV)) { + GlobalValueSummary::GVFlags Flags(GlobalValue::InternalLinkage, + true, true); + std::unique_ptr Summary = + llvm::make_unique(Flags, 0); + Summary->setHasSection(); + Index.addGlobalValueSummary(Name, std::move(Summary)); + } else { + // FIXME: Need to handle aliases defined in module asm?! + GlobalValueSummary::GVFlags Flags(GlobalValue::InternalLinkage, + true, true); + std::unique_ptr Summary = + llvm::make_unique(Flags); + Summary->setHasSection(); + Index.addGlobalValueSummary(Name, std::move(Summary)); + } + } else { + // Check for local used in module asm + GlobalValue *GV = M.getNamedValue(Name); + if (!GV || !GV->hasLocalLinkage()) + return; + auto *Summary = Index.getGlobalValueSummary(*GV); + assert(Summary); + Summary->setHasSection(); + } + }); + } + return Index; } @@ -271,6 +352,7 @@ // into this module, which is also necessary to avoid needing to rename // in case of a name clash between a local in this module and an imported // global. + // // FIXME: If we find we need a finer-grained approach of preventing promotion // and renaming of just the functions using inline assembly we will need to: // - Add flag in the function summaries to identify those with inline asm. @@ -281,6 +363,24 @@ // alias to the original name in the current module (even if we don't // export the function using those values in inline asm, another function // with a reference could be exported). + + // We similarly prevent importing from modules containing module-level inline + // assembly that defines a local value. These would also requiring renaming + // on import. + if (!M.getModuleInlineAsm().empty()) { + bool LocalIsDefined = false; + object::IRObjectFile::CollectAsmUndefinedRefs( + Triple(M.getTargetTriple()), M.getModuleInlineAsm(), + [&LocalIsDefined](StringRef Name, object::BasicSymbolRef::Flags Flags) { + if (!(Flags & (object::BasicSymbolRef::SF_Weak || + object::BasicSymbolRef::SF_Global))) + LocalIsDefined = true; + }); + + if (LocalIsDefined) + return false; + } + SmallPtrSet Used; collectUsedGlobalVariables(M, Used, /*CompilerUsed*/ false); bool LocalIsUsed = Index: lib/Bitcode/Writer/BitcodeWriter.cpp =================================================================== --- lib/Bitcode/Writer/BitcodeWriter.cpp +++ lib/Bitcode/Writer/BitcodeWriter.cpp @@ -283,7 +283,7 @@ unsigned FSCallsAbbrev, unsigned FSCallsProfileAbbrev, const Function &F); - void writeModuleLevelReferences(const GlobalVariable &V, + void writeModuleLevelReferences(const GlobalValue &V, SmallVector &NameVals, unsigned FSModRefsAbbrev); void writePerModuleGlobalValueSummary(); @@ -3325,15 +3325,17 @@ // Collect the global value references in the given variable's initializer, // and emit them in a summary record. void ModuleBitcodeWriter::writeModuleLevelReferences( - const GlobalVariable &V, SmallVector &NameVals, + const GlobalValue &V, SmallVector &NameVals, unsigned FSModRefsAbbrev) { - // Only interested in recording variable defs in the summary. - if (V.isDeclaration()) + auto *Summary = Index->getGlobalValueSummary(V, true, false); + if (!Summary) { + // Declaration without a def in module level asm + assert(V.isDeclaration()); return; + } NameVals.push_back(VE.getValueID(&V)); - NameVals.push_back(getEncodedGVSummaryFlags(V)); - auto *Summary = Index->getGlobalValueSummary(V); GlobalVarSummary *VS = cast(Summary); + NameVals.push_back(getEncodedGVSummaryFlags(VS->flags())); unsigned SizeBeforeRefs = NameVals.size(); for (auto &RI : VS->refs()) @@ -3409,14 +3411,17 @@ // Iterate over the list of functions instead of the Index to // ensure the ordering is stable. for (const Function &F : M) { - if (F.isDeclaration()) - continue; // Summary emission does not support anonymous functions, they have to // renamed using the anonymous function renaming pass. if (!F.hasName()) report_fatal_error("Unexpected anonymous function when writing summary"); - auto *Summary = Index->getGlobalValueSummary(F); + auto *Summary = Index->getGlobalValueSummary(F, true, false); + if (!Summary) { + // Declaration without a def in module level asm + assert(F.isDeclaration()); + continue; + } writePerModuleFunctionSummaryRecord(NameVals, Summary, VE.getValueID(&F), FSCallsAbbrev, FSCallsProfileAbbrev, F); } @@ -3434,7 +3439,9 @@ auto AliasId = VE.getValueID(&A); auto AliaseeId = VE.getValueID(Aliasee); NameVals.push_back(AliasId); - NameVals.push_back(getEncodedGVSummaryFlags(A)); + auto *Summary = Index->getGlobalValueSummary(A); + AliasSummary *AS = cast(Summary); + NameVals.push_back(getEncodedGVSummaryFlags(AS->flags())); NameVals.push_back(AliaseeId); Stream.EmitRecord(bitc::FS_ALIAS, NameVals, FSAliasAbbrev); NameVals.clear(); Index: lib/IR/ModuleSummaryIndex.cpp =================================================================== --- lib/IR/ModuleSummaryIndex.cpp +++ lib/IR/ModuleSummaryIndex.cpp @@ -94,10 +94,11 @@ } } -GlobalValueSummary * -ModuleSummaryIndex::getGlobalValueSummary(uint64_t ValueGUID, - bool PerModuleIndex) const { +GlobalValueSummary *ModuleSummaryIndex::getGlobalValueSummary( + uint64_t ValueGUID, bool PerModuleIndex, bool AssertExists) const { auto SummaryList = findGlobalValueSummaryList(ValueGUID); + if (!AssertExists && SummaryList == end()) + return nullptr; assert(SummaryList != end() && "GlobalValue not found in index"); assert((!PerModuleIndex || SummaryList->second.size() == 1) && "Expected a single entry per global value in per-module index"); Index: lib/Object/IRObjectFile.cpp =================================================================== --- lib/Object/IRObjectFile.cpp +++ lib/Object/IRObjectFile.cpp @@ -54,8 +54,7 @@ std::string Err; const Target *T = TargetRegistry::lookupTarget(TT.str(), Err); - if (!T) - return; + assert(T && T->hasMCAsmParser()); std::unique_ptr MRI(T->createMCRegInfo(TT.str())); if (!MRI) Index: lib/Transforms/Utils/FunctionImportUtils.cpp =================================================================== --- lib/Transforms/Utils/FunctionImportUtils.cpp +++ lib/Transforms/Utils/FunctionImportUtils.cpp @@ -67,9 +67,9 @@ if (GVar && GVar->isConstant() && GVar->hasGlobalUnnamedAddr()) return false; - if (GVar && GVar->hasSection()) - // Some sections like "__DATA,__cfstring" are "magic" and promotion is not - // allowed. Just disable promotion on any GVar with sections right now. + auto *Summary = ImportIndex.getGlobalValueSummary(SGV->getGUID()); + assert(Summary && "Missing summary for global value"); + if (Summary->hasSection()) return false; // Eventually we only need to promote functions in the exporting module that @@ -78,14 +78,14 @@ return true; } -std::string FunctionImportGlobalProcessing::getName(const GlobalValue *SGV) { +std::string FunctionImportGlobalProcessing::getName(const GlobalValue *SGV, + bool DoPromote) { // For locals that must be promoted to global scope, ensure that // the promoted name uniquely identifies the copy in the original module, // using the ID assigned during combined index creation. When importing, // we rename all locals (not just those that are promoted) in order to // avoid naming conflicts between locals imported from different modules. - if (SGV->hasLocalLinkage() && - (doPromoteLocalToGlobal(SGV) || isPerformingImport())) + if (SGV->hasLocalLinkage() && (DoPromote || isPerformingImport())) return ModuleSummaryIndex::getGlobalNameForLocal( SGV->getName(), ImportIndex.getModuleHash(SGV->getParent()->getModuleIdentifier())); @@ -93,13 +93,14 @@ } GlobalValue::LinkageTypes -FunctionImportGlobalProcessing::getLinkage(const GlobalValue *SGV) { +FunctionImportGlobalProcessing::getLinkage(const GlobalValue *SGV, + bool DoPromote) { // Any local variable that is referenced by an exported function needs // to be promoted to global scope. Since we don't currently know which // functions reference which local variables/functions, we must treat // all as potentially exported if this module is exporting anything. if (isModuleExporting()) { - if (SGV->hasLocalLinkage() && doPromoteLocalToGlobal(SGV)) + if (SGV->hasLocalLinkage() && DoPromote) return GlobalValue::ExternalLinkage; return SGV->getLinkage(); } @@ -164,7 +165,7 @@ case GlobalValue::PrivateLinkage: // If we are promoting the local to global scope, it is handled // similarly to a normal externally visible global. - if (doPromoteLocalToGlobal(SGV)) { + if (DoPromote) { if (doImportAsDefinition(SGV) && !dyn_cast(SGV)) return GlobalValue::AvailableExternallyLinkage; else @@ -190,14 +191,15 @@ } void FunctionImportGlobalProcessing::processGlobalForThinLTO(GlobalValue &GV) { + bool DoPromote = false; if (GV.hasLocalLinkage() && - (doPromoteLocalToGlobal(&GV) || isPerformingImport())) { - GV.setName(getName(&GV)); - GV.setLinkage(getLinkage(&GV)); + ((DoPromote = doPromoteLocalToGlobal(&GV)) || isPerformingImport())) { + GV.setName(getName(&GV, DoPromote)); + GV.setLinkage(getLinkage(&GV, DoPromote)); if (!GV.hasLocalLinkage()) GV.setVisibility(GlobalValue::HiddenVisibility); } else - GV.setLinkage(getLinkage(&GV)); + GV.setLinkage(getLinkage(&GV, DoPromote)); // Remove functions imported as available externally defs from comdats, // as this is a declaration for the linker, and will be dropped eventually. @@ -214,14 +216,6 @@ } void FunctionImportGlobalProcessing::processGlobalsForThinLTO() { - if (!moduleCanBeRenamedForThinLTO(M)) { - // We would have blocked importing from this module by suppressing index - // generation. We still may be able to import into this module though. - assert(!isPerformingImport() && - "Should have blocked importing from module with local used in ASM"); - return; - } - for (GlobalVariable &GV : M.globals()) processGlobalForThinLTO(GV); for (Function &SF : M) Index: test/LTO/X86/current-section.ll =================================================================== --- test/LTO/X86/current-section.ll +++ test/LTO/X86/current-section.ll @@ -2,4 +2,7 @@ ; RUN: llvm-lto -o %t2 %t1 ; REQUIRES: default_triple +target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128" +target triple = "x86_64-unknown-linux-gnu" + module asm ".align 4" Index: test/ThinLTO/X86/Inputs/module_asm2.ll =================================================================== --- /dev/null +++ test/ThinLTO/X86/Inputs/module_asm2.ll @@ -0,0 +1,10 @@ +target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128" +target triple = "x86_64-unknown-linux-gnu" + +define i32 @main({ i64, { i64, i8* }* } %unnamed) #0 { + %1 = call i32 @func1() #1 + %2 = call i32 @func2() #1 + ret i32 %1 +} +declare i32 @func1() #1 +declare i32 @func2() #1 Index: test/ThinLTO/X86/module_asm2.ll =================================================================== --- /dev/null +++ test/ThinLTO/X86/module_asm2.ll @@ -0,0 +1,46 @@ +; RUN: opt -module-summary %s -o %t1.bc +; RUN: opt -module-summary %p/Inputs/module_asm2.ll -o %t2.bc + +; RUN: llvm-lto -thinlto-action=run -exported-symbol=main -exported-symbol=func1 -exported-symbol=func2 %t1.bc %t2.bc +; RUN: llvm-nm %t1.bc.thinlto.o | FileCheck %s --check-prefix=NM0 +; RUN: llvm-nm %t2.bc.thinlto.o | FileCheck %s --check-prefix=NM1 + +; RUN: llvm-lto2 %t1.bc %t2.bc -o %t.o -save-temps \ +; RUN: -r=%t1.bc,foo,plx \ +; RUN: -r=%t1.bc,b,pl \ +; RUN: -r=%t1.bc,func1,pl \ +; RUN: -r=%t1.bc,func2,pl \ +; RUN: -r=%t2.bc,main,plx \ +; RUN: -r=%t2.bc,func1,l \ +; RUN: -r=%t2.bc,func2,l +; RUN: llvm-nm %t.o.0 | FileCheck %s --check-prefix=NM0 +; RUN: llvm-nm %t.o.1 | FileCheck %s --check-prefix=NM1 + +; NM0-DAG: t foo +; NM1-NOT: foo +; NM1-NOT: b + +target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128" +target triple = "x86_64-unknown-linux-gnu" + +@b = internal global i32 1, align 4 + +module asm "\09.text" +module asm "\09.type\09foo,@function" +module asm "foo:" +module asm "\09movl b, %eax" +module asm "\09ret " +module asm "\09.size\09foo, .-foo" +module asm "" + +declare i16 @foo() #0 + +define i32 @func1() #1 { + call i16 @foo() + ret i32 1 +} + +define i32 @func2() #1 { + %1 = load i32, i32* @b, align 4 + ret i32 %1 +} Index: tools/opt/opt.cpp =================================================================== --- tools/opt/opt.cpp +++ tools/opt/opt.cpp @@ -364,6 +364,7 @@ InitializeAllTargets(); InitializeAllTargetMCs(); InitializeAllAsmPrinters(); + InitializeAllAsmParsers(); // Initialize passes PassRegistry &Registry = *PassRegistry::getPassRegistry();