[llvm] r286329 - Revert "[ThinLTO] Prevent exporting of locals used/defined in module level asm"

Mehdi Amini via llvm-commits llvm-commits at lists.llvm.org
Tue Nov 8 17:45:13 PST 2016


Author: mehdi_amini
Date: Tue Nov  8 19:45:13 2016
New Revision: 286329

URL: http://llvm.org/viewvc/llvm-project?rev=286329&view=rev
Log:
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.

Removed:
    llvm/trunk/test/ThinLTO/X86/Inputs/module_asm2.ll
    llvm/trunk/test/ThinLTO/X86/module_asm2.ll
Modified:
    llvm/trunk/include/llvm/Support/TargetRegistry.h
    llvm/trunk/lib/Analysis/LLVMBuild.txt
    llvm/trunk/lib/Analysis/ModuleSummaryAnalysis.cpp
    llvm/trunk/lib/Bitcode/Writer/BitcodeWriter.cpp
    llvm/trunk/lib/Object/IRObjectFile.cpp
    llvm/trunk/test/LTO/X86/current-section.ll
    llvm/trunk/tools/opt/opt.cpp

Modified: llvm/trunk/include/llvm/Support/TargetRegistry.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/Support/TargetRegistry.h?rev=286329&r1=286328&r2=286329&view=diff
==============================================================================
--- llvm/trunk/include/llvm/Support/TargetRegistry.h (original)
+++ llvm/trunk/include/llvm/Support/TargetRegistry.h Tue Nov  8 19:45:13 2016
@@ -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
   /// @{

Modified: llvm/trunk/lib/Analysis/LLVMBuild.txt
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Analysis/LLVMBuild.txt?rev=286329&r1=286328&r2=286329&view=diff
==============================================================================
--- llvm/trunk/lib/Analysis/LLVMBuild.txt (original)
+++ llvm/trunk/lib/Analysis/LLVMBuild.txt Tue Nov  8 19:45:13 2016
@@ -19,4 +19,4 @@
 type = Library
 name = Analysis
 parent = Libraries
-required_libraries = Core Support ProfileData Object
+required_libraries = Core Support ProfileData

Modified: llvm/trunk/lib/Analysis/ModuleSummaryAnalysis.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Analysis/ModuleSummaryAnalysis.cpp?rev=286329&r1=286328&r2=286329&view=diff
==============================================================================
--- llvm/trunk/lib/Analysis/ModuleSummaryAnalysis.cpp (original)
+++ llvm/trunk/lib/Analysis/ModuleSummaryAnalysis.cpp Tue Nov  8 19:45:13 2016
@@ -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::buildModuleSumm
     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<GlobalValue *, 8> 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<GlobalValue *, 8> 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<GlobalValue *, 8> LocalsUsed;
   for (auto *V : Used) {
     if (V->hasLocalLinkage())
       LocalsUsed.insert(V);
@@ -256,47 +244,6 @@ ModuleSummaryIndex llvm::buildModuleSumm
     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<Function>(GV)) {
-            std::unique_ptr<FunctionSummary> Summary =
-                llvm::make_unique<FunctionSummary>(GVFlags, 0);
-            Summary->setNoRename();
-            Index.addGlobalValueSummary(Name, std::move(Summary));
-          } else {
-            std::unique_ptr<GlobalVarSummary> Summary =
-                llvm::make_unique<GlobalVarSummary>(GVFlags);
-            Summary->setNoRename();
-            Index.addGlobalValueSummary(Name, std::move(Summary));
-          }
-        });
-  }
-
   return Index;
 }
 

Modified: llvm/trunk/lib/Bitcode/Writer/BitcodeWriter.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Bitcode/Writer/BitcodeWriter.cpp?rev=286329&r1=286328&r2=286329&view=diff
==============================================================================
--- llvm/trunk/lib/Bitcode/Writer/BitcodeWriter.cpp (original)
+++ llvm/trunk/lib/Bitcode/Writer/BitcodeWriter.cpp Tue Nov  8 19:45:13 2016
@@ -3327,16 +3327,11 @@ void ModuleBitcodeWriter::writePerModule
 void ModuleBitcodeWriter::writeModuleLevelReferences(
     const GlobalVariable &V, SmallVector<uint64_t, 64> &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<GlobalVarSummary>(Summary);
   NameVals.push_back(getEncodedGVSummaryFlags(VS->flags()));
 
@@ -3414,20 +3409,14 @@ void ModuleBitcodeWriter::writePerModule
   // 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);
   }

Modified: llvm/trunk/lib/Object/IRObjectFile.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Object/IRObjectFile.cpp?rev=286329&r1=286328&r2=286329&view=diff
==============================================================================
--- llvm/trunk/lib/Object/IRObjectFile.cpp (original)
+++ llvm/trunk/lib/Object/IRObjectFile.cpp Tue Nov  8 19:45:13 2016
@@ -54,7 +54,8 @@ void IRObjectFile::CollectAsmUndefinedRe
 
   std::string Err;
   const Target *T = TargetRegistry::lookupTarget(TT.str(), Err);
-  assert(T && T->hasMCAsmParser());
+  if (!T)
+    return;
 
   std::unique_ptr<MCRegisterInfo> MRI(T->createMCRegInfo(TT.str()));
   if (!MRI)

Modified: llvm/trunk/test/LTO/X86/current-section.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/LTO/X86/current-section.ll?rev=286329&r1=286328&r2=286329&view=diff
==============================================================================
--- llvm/trunk/test/LTO/X86/current-section.ll (original)
+++ llvm/trunk/test/LTO/X86/current-section.ll Tue Nov  8 19:45:13 2016
@@ -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"

Removed: llvm/trunk/test/ThinLTO/X86/Inputs/module_asm2.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/ThinLTO/X86/Inputs/module_asm2.ll?rev=286328&view=auto
==============================================================================
--- llvm/trunk/test/ThinLTO/X86/Inputs/module_asm2.ll (original)
+++ llvm/trunk/test/ThinLTO/X86/Inputs/module_asm2.ll (removed)
@@ -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

Removed: llvm/trunk/test/ThinLTO/X86/module_asm2.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/ThinLTO/X86/module_asm2.ll?rev=286328&view=auto
==============================================================================
--- llvm/trunk/test/ThinLTO/X86/module_asm2.ll (original)
+++ llvm/trunk/test/ThinLTO/X86/module_asm2.ll (removed)
@@ -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"
-
- at b = internal global i32 1, align 4
- at x = internal global i32 1, align 4
-
- at llvm.compiler.used = appending global [1 x i8*] [i8* bitcast (i32* @b to i8*)], section "llvm.metadata"
- at 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, at 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
-}

Modified: llvm/trunk/tools/opt/opt.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/tools/opt/opt.cpp?rev=286329&r1=286328&r2=286329&view=diff
==============================================================================
--- llvm/trunk/tools/opt/opt.cpp (original)
+++ llvm/trunk/tools/opt/opt.cpp Tue Nov  8 19:45:13 2016
@@ -364,7 +364,6 @@ int main(int argc, char **argv) {
   InitializeAllTargets();
   InitializeAllTargetMCs();
   InitializeAllAsmPrinters();
-  InitializeAllAsmParsers();
 
   // Initialize passes
   PassRegistry &Registry = *PassRegistry::getPassRegistry();




More information about the llvm-commits mailing list