]> granicus.if.org Git - llvm/commitdiff
Restore "[ThinLTO] Prevent exporting of locals used/defined in module level asm"
authorTeresa Johnson <tejohnson@google.com>
Mon, 14 Nov 2016 17:12:32 +0000 (17:12 +0000)
committerTeresa Johnson <tejohnson@google.com>
Mon, 14 Nov 2016 17:12:32 +0000 (17:12 +0000)
This restores the rest of r286297 (part was restored in r286475).
Specifically, it restores the part requiring adding a dependency from
the Analysis to Object library (downstream use changed to correctly
model split BitReader vs BitWriter libraries).

Original description of this part of patch follows:

Module level asm may also contain defs of values. We need to prevent
export of any refs to local values defined in module level asm (e.g. a
ref in normal IR), since that also requires renaming/promotion of the
local. To do that, the summary index builder looks at all values in the
module level asm string that are not marked Weak or Global, which is
exactly the set of locals that are defined. A summary is created for
each of these local defs and flagged as NoRename.

This required adding handling to the BitcodeWriter to look at GV
declarations to see if they have a summary (rather than skipping them
all).

Finally, added an assert to IRObjectFile::CollectAsmUndefinedRefs to
ensure that an MCAsmParser is available, otherwise the module asm parse
would silently fail. Initialized the asm parser in the opt tool for use
in testing this fix.

Fixes PR30610.

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

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

index f868c07df182896f9e0b3d5ebd7032ce083197f8..954cdb13abafa435183854db2ce19f4a1ec06725 100644 (file)
@@ -280,6 +280,9 @@ 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
   /// @{
index 08af5f37700d652a308b5e5456a34a70781153a4..15c757b48f76c8a6e0452e009a8cbea5d5600681 100644 (file)
@@ -19,4 +19,4 @@
 type = Library
 name = Analysis
 parent = Libraries
-required_libraries = Core Support ProfileData
+required_libraries = Core Support ProfileData Object
index 0c07e1d878f68150c5dcdbe5887259ee7f00882c..133bb3f9c9ded35614288a1195b5f5cf6c20a39f 100644 (file)
@@ -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;
 
@@ -257,6 +259,49 @@ 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,
+              /* HasInlineAsmMaybeReferencingInternal */ false,
+              /* 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;
 }
 
index 80db5b2e061da2d89bc1bca82808c4e82d29c34c..c62cd1f20429046ad59a6b82c76d1f05060d50ab 100644 (file)
@@ -3331,11 +3331,16 @@ void ModuleBitcodeWriter::writePerModuleFunctionSummaryRecord(
 void ModuleBitcodeWriter::writeModuleLevelReferences(
     const GlobalVariable &V, SmallVector<uint64_t, 64> &NameVals,
     unsigned FSModRefsAbbrev) {
-  // Only interested in recording variable defs in the summary.
-  if (V.isDeclaration())
+  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());
     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()));
 
@@ -3413,14 +3418,20 @@ 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 *Summary = Index->getGlobalValueSummary(F);
+    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();
     writePerModuleFunctionSummaryRecord(NameVals, Summary, VE.getValueID(&F),
                                         FSCallsAbbrev, FSCallsProfileAbbrev, F);
   }
index 535ddb919354065ea567cd02d698e4c1994ed085..3aa9b755705a968d5bf16f6b9269260c1ff0afd2 100644 (file)
@@ -54,8 +54,7 @@ void IRObjectFile::CollectAsmUndefinedRefs(
 
   std::string Err;
   const Target *T = TargetRegistry::lookupTarget(TT.str(), Err);
-  if (!T)
-    return;
+  assert(T && T->hasMCAsmParser());
 
   std::unique_ptr<MCRegisterInfo> MRI(T->createMCRegInfo(TT.str()));
   if (!MRI)
index 49eee49ae623bd606e8bf4b1564dc8c44b29e931..69f8bfaac3b0c84213ffeb88baf2fd72df544732 100644 (file)
@@ -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 3ec0945d65c08f1d5cf01f5a89fd57081d22174f..7ad73cb2134cbe90858ed9b18ae378b10259c250 100644 (file)
 ; NM0-DAG: T func2
 ; NM0-DAG: T func3
 
-; Ensure that b and x are likewise not exported (imported as refs
+; 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 func2 and func3 should not have been
 ; imported.
-; FIXME: Likewise, foo should not be exported, along with referencing function
-; func1. However, this relies on being able to add a dependence from
-; libAnalysis to libObject, which is currently blocked (see revert of
-; r286297).
+; 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
 
index ef4f975fb794c6508167ca450c74a1c8939c18ca..0a42c13898ab3bdc816e0688751e77e2d80cd8ff 100644 (file)
@@ -364,6 +364,7 @@ int main(int argc, char **argv) {
   InitializeAllTargets();
   InitializeAllTargetMCs();
   InitializeAllAsmPrinters();
+  InitializeAllAsmParsers();
 
   // Initialize passes
   PassRegistry &Registry = *PassRegistry::getPassRegistry();