From: Mehdi Amini Date: Wed, 9 Nov 2016 01:45:13 +0000 (+0000) Subject: Revert "[ThinLTO] Prevent exporting of locals used/defined in module level asm" X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=129c9fcdb8e103a5f9c71d5d1d73277600ef80a6;p=llvm Revert "[ThinLTO] Prevent exporting of locals used/defined in module level asm" This reverts commit r286297. Introduces a dependency from libAnalysis to libObject, which I missed during the review. git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@286329 91177308-0d34-0410-b5e6-96231b3b80d8 --- diff --git a/include/llvm/Support/TargetRegistry.h b/include/llvm/Support/TargetRegistry.h index 954cdb13aba..f868c07df18 100644 --- a/include/llvm/Support/TargetRegistry.h +++ b/include/llvm/Support/TargetRegistry.h @@ -280,9 +280,6 @@ public: /// 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 /// @{ diff --git a/lib/Analysis/LLVMBuild.txt b/lib/Analysis/LLVMBuild.txt index 15c757b48f7..08af5f37700 100644 --- a/lib/Analysis/LLVMBuild.txt +++ b/lib/Analysis/LLVMBuild.txt @@ -19,4 +19,4 @@ type = Library name = Analysis parent = Libraries -required_libraries = Core Support ProfileData Object +required_libraries = Core Support ProfileData diff --git a/lib/Analysis/ModuleSummaryAnalysis.cpp b/lib/Analysis/ModuleSummaryAnalysis.cpp index 5cf4a895c91..09d18eb3da1 100644 --- a/lib/Analysis/ModuleSummaryAnalysis.cpp +++ b/lib/Analysis/ModuleSummaryAnalysis.cpp @@ -13,7 +13,6 @@ //===----------------------------------------------------------------------===// #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" @@ -25,7 +24,6 @@ #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; @@ -196,22 +194,12 @@ ModuleSummaryIndex llvm::buildModuleSummaryIndex( ProfileSummaryInfo *PSI) { ModuleSummaryIndex Index; - // Identify the local values in the llvm.used and llvm.compiler.used sets, - // which should not be exported as they would then require renaming and - // promotion, but we may have opaque uses e.g. in inline asm. We collect them - // here because we use this information to mark functions containing inline - // assembly calls as not importable. - SmallPtrSet LocalsUsed; + // Identify the local values in the llvm.used set, which should not be + // exported as they would then require renaming and promotion, but we + // may have opaque uses e.g. in inline asm. SmallPtrSet Used; - // First collect those in the llvm.used set. collectUsedGlobalVariables(M, Used, /*CompilerUsed*/ false); - for (auto *V : Used) { - if (V->hasLocalLinkage()) - LocalsUsed.insert(V); - } - Used.clear(); - // Next collect those in the llvm.compiler.used set. - collectUsedGlobalVariables(M, Used, /*CompilerUsed*/ true); + SmallPtrSet LocalsUsed; for (auto *V : Used) { if (V->hasLocalLinkage()) LocalsUsed.insert(V); @@ -256,47 +244,6 @@ ModuleSummaryIndex llvm::buildModuleSummaryIndex( Summary->setNoRename(); } - if (!M.getModuleInlineAsm().empty()) { - // Collect the local values defined by module level asm, and set up - // summaries for these symbols so that they can be marked as NoRename, - // to prevent export of any use of them in regular IR that would require - // renaming within the module level asm. Note we don't need to create a - // summary for weak or global defs, as they don't need to be flagged as - // NoRename, and defs in module level asm can't be imported anyway. - // Also, any values used but not defined within module level asm should - // be listed on the llvm.used or llvm.compiler.used global and marked as - // referenced from there. - // FIXME: Rename CollectAsmUndefinedRefs to something more general, as we - // are also using it to find the file-scope locals defined in module asm. - object::IRObjectFile::CollectAsmUndefinedRefs( - Triple(M.getTargetTriple()), M.getModuleInlineAsm(), - [&M, &Index](StringRef Name, object::BasicSymbolRef::Flags Flags) { - // Symbols not marked as Weak or Global are local definitions. - if (Flags & (object::BasicSymbolRef::SF_Weak || - object::BasicSymbolRef::SF_Global)) - return; - GlobalValue *GV = M.getNamedValue(Name); - if (!GV) - return; - assert(GV->isDeclaration() && "Def in module asm already has definition"); - GlobalValueSummary::GVFlags GVFlags(GlobalValue::InternalLinkage, - /* NoRename */ true, - /*IsNotViableToInline */ true); - // Create the appropriate summary type. - if (isa(GV)) { - std::unique_ptr Summary = - llvm::make_unique(GVFlags, 0); - Summary->setNoRename(); - Index.addGlobalValueSummary(Name, std::move(Summary)); - } else { - std::unique_ptr Summary = - llvm::make_unique(GVFlags); - Summary->setNoRename(); - Index.addGlobalValueSummary(Name, std::move(Summary)); - } - }); - } - return Index; } diff --git a/lib/Bitcode/Writer/BitcodeWriter.cpp b/lib/Bitcode/Writer/BitcodeWriter.cpp index 7b49aac006f..8de61bc0b7c 100644 --- a/lib/Bitcode/Writer/BitcodeWriter.cpp +++ b/lib/Bitcode/Writer/BitcodeWriter.cpp @@ -3327,16 +3327,11 @@ void ModuleBitcodeWriter::writePerModuleFunctionSummaryRecord( void ModuleBitcodeWriter::writeModuleLevelReferences( const GlobalVariable &V, SmallVector &NameVals, unsigned FSModRefsAbbrev) { - auto Summaries = - Index->findGlobalValueSummaryList(GlobalValue::getGUID(V.getName())); - if (Summaries == Index->end()) { - // Only declarations should not have a summary (a declaration might however - // have a summary if the def was in module level asm). - assert(V.isDeclaration()); + // Only interested in recording variable defs in the summary. + if (V.isDeclaration()) return; - } - auto *Summary = Summaries->second.front().get(); NameVals.push_back(VE.getValueID(&V)); + auto *Summary = Index->getGlobalValueSummary(V); GlobalVarSummary *VS = cast(Summary); NameVals.push_back(getEncodedGVSummaryFlags(VS->flags())); @@ -3414,20 +3409,14 @@ void ModuleBitcodeWriter::writePerModuleGlobalValueSummary() { // 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 Summaries = - Index->findGlobalValueSummaryList(GlobalValue::getGUID(F.getName())); - if (Summaries == Index->end()) { - // Only declarations should not have a summary (a declaration might - // however have a summary if the def was in module level asm). - assert(F.isDeclaration()); - continue; - } - auto *Summary = Summaries->second.front().get(); + auto *Summary = Index->getGlobalValueSummary(F); writePerModuleFunctionSummaryRecord(NameVals, Summary, VE.getValueID(&F), FSCallsAbbrev, FSCallsProfileAbbrev, F); } diff --git a/lib/Object/IRObjectFile.cpp b/lib/Object/IRObjectFile.cpp index d9474b94b30..8769f8fe54a 100644 --- a/lib/Object/IRObjectFile.cpp +++ b/lib/Object/IRObjectFile.cpp @@ -54,7 +54,8 @@ void IRObjectFile::CollectAsmUndefinedRefs( std::string Err; const Target *T = TargetRegistry::lookupTarget(TT.str(), Err); - assert(T && T->hasMCAsmParser()); + if (!T) + return; std::unique_ptr MRI(T->createMCRegInfo(TT.str())); if (!MRI) diff --git a/test/LTO/X86/current-section.ll b/test/LTO/X86/current-section.ll index 69f8bfaac3b..49eee49ae62 100644 --- a/test/LTO/X86/current-section.ll +++ b/test/LTO/X86/current-section.ll @@ -2,7 +2,4 @@ ; 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" diff --git a/test/ThinLTO/X86/Inputs/module_asm2.ll b/test/ThinLTO/X86/Inputs/module_asm2.ll deleted file mode 100644 index e823277da67..00000000000 --- a/test/ThinLTO/X86/Inputs/module_asm2.ll +++ /dev/null @@ -1,12 +0,0 @@ -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 - %3 = call i32 @func3() #1 - ret i32 %1 -} -declare i32 @func1() #1 -declare i32 @func2() #1 -declare i32 @func3() #1 diff --git a/test/ThinLTO/X86/module_asm2.ll b/test/ThinLTO/X86/module_asm2.ll deleted file mode 100644 index 90ef75fbc5e..00000000000 --- a/test/ThinLTO/X86/module_asm2.ll +++ /dev/null @@ -1,84 +0,0 @@ -; Test to ensure that uses and defs in module level asm are handled -; appropriately. Specifically, we should conservatively block importing -; of any references to these values, as they can't be renamed. -; 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 -exported-symbol=func3 %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,x,pl \ -; RUN: -r=%t1.bc,func1,pl \ -; RUN: -r=%t1.bc,func2,pl \ -; RUN: -r=%t1.bc,func3,pl \ -; RUN: -r=%t2.bc,main,plx \ -; RUN: -r=%t2.bc,func1,l \ -; RUN: -r=%t2.bc,func2,l \ -; RUN: -r=%t2.bc,func3,l -; RUN: llvm-nm %t.o.0 | FileCheck %s --check-prefix=NM0 -; RUN: llvm-nm %t.o.1 | FileCheck %s --check-prefix=NM1 - -; Check that local values b and x, which are referenced on -; llvm.used and llvm.compiler.used, respectively, are not promoted. -; Similarly, foo which is defined in module level asm should not be -; promoted. -; NM0-DAG: d b -; NM0-DAG: d x -; NM0-DAG: t foo -; NM0-DAG: T func1 -; NM0-DAG: T func2 -; NM0-DAG: T func3 - -; Ensure that foo, b and x are likewise not exported (imported as refs -; into the other module), since they can't be promoted. Additionally, -; referencing functions func1, func2 and func3 should not have been -; imported. -; NM1-NOT: foo -; NM1-NOT: b -; NM1-NOT: x -; NM1-DAG: U func1 -; NM1-DAG: U func2 -; NM1-DAG: U func3 -; NM1-DAG: T main -; NM1-NOT: foo -; NM1-NOT: b -; NM1-NOT: x - -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 -@x = internal global i32 1, align 4 - -@llvm.compiler.used = appending global [1 x i8*] [i8* bitcast (i32* @b to i8*)], section "llvm.metadata" -@llvm.used = appending global [1 x i8*] [i8* bitcast (i32* @x to i8*)], section "llvm.metadata" - -module asm "\09.text" -module asm "\09.type\09foo,@function" -module asm "foo:" -module asm "\09movl b, %eax" -module asm "\09movl x, %edx" -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 -} - -define i32 @func3() #1 { - %1 = load i32, i32* @x, align 4 - ret i32 %1 -} diff --git a/tools/opt/opt.cpp b/tools/opt/opt.cpp index 0a42c13898a..ef4f975fb79 100644 --- a/tools/opt/opt.cpp +++ b/tools/opt/opt.cpp @@ -364,7 +364,6 @@ int main(int argc, char **argv) { InitializeAllTargets(); InitializeAllTargetMCs(); InitializeAllAsmPrinters(); - InitializeAllAsmParsers(); // Initialize passes PassRegistry &Registry = *PassRegistry::getPassRegistry();