[ThinLTO] Use per-summary flag to prevent exporting locals used in inline asm

Summary:
Instead of using the workaround of suppressing the entire index for
modules that call inline asm that may reference locals, use the
NoRename flag on the summary for any locals in the llvm.used set, and
add a reference edge from any functions containing inline asm.

This avoids issues from having no summaries despite the module defining
global values, which was preventing more aggressive index-based
optimization. It will be followed by a subsequent patch to make a
similar fix for local references in module level asm (to fix PR30610).

Reviewers: mehdi_amini

Subscribers: llvm-commits

Differential Revision: https://reviews.llvm.org/D26121

llvm-svn: 285513

diff --git a/llvm/include/llvm/Analysis/ModuleSummaryAnalysis.h b/llvm/include/llvm/Analysis/ModuleSummaryAnalysis.h
index c8adc3bf81ac748557b6639364d5f8a787a372b9..617c3d8f17a28f90b1db1bf0d9374da208835939 100644 (file)
--- a/llvm/include/llvm/Analysis/ModuleSummaryAnalysis.h
+++ b/llvm/include/llvm/Analysis/ModuleSummaryAnalysis.h
@@ -70,11 +70,6 @@ public:
 // object for the module, to be written to bitcode or LLVM assembly.
 //
 ModulePass *createModuleSummaryIndexWrapperPass();
-
-/// Returns true if \p M is eligible for ThinLTO promotion.
-///
-/// Currently we check if it has any any InlineASM that uses an internal symbol.
-bool moduleCanBeRenamedForThinLTO(const Module &M);
 }
 
 #endif

diff --git a/llvm/include/llvm/IR/ModuleSummaryIndex.h b/llvm/include/llvm/IR/ModuleSummaryIndex.h
index 1ceb9eb32cbd398858073d2d8cb7d1500c7d4124..b3775904e75746e4bf94522afc04023095704300 100644 (file)
--- a/llvm/include/llvm/IR/ModuleSummaryIndex.h
+++ b/llvm/include/llvm/IR/ModuleSummaryIndex.h
@@ -194,6 +194,10 @@ public:
   /// possibly referenced from inline assembly, etc).
   bool noRename() const { return Flags.NoRename; }
 
+  /// Flag that this global value cannot be renamed (in a specific section,
+  /// possibly referenced from inline assembly, etc).
+  void setNoRename() { Flags.NoRename = 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); }

diff --git a/llvm/lib/Analysis/ModuleSummaryAnalysis.cpp b/llvm/lib/Analysis/ModuleSummaryAnalysis.cpp
index f13ae844442ab3a94f8b3f33196205b035a35999..09d18eb3da13417a08507c8e6147b6fb3099041a 100644 (file)
--- a/llvm/lib/Analysis/ModuleSummaryAnalysis.cpp
+++ b/llvm/lib/Analysis/ModuleSummaryAnalysis.cpp
@@ -77,7 +77,8 @@ static CalleeInfo::HotnessType getHotness(uint64_t ProfileCount,
 
 static void computeFunctionSummary(ModuleSummaryIndex &Index, const Module &M,
                                    const Function &F, BlockFrequencyInfo *BFI,
-                                   ProfileSummaryInfo *PSI) {
+                                   ProfileSummaryInfo *PSI,
+                                   SmallPtrSetImpl<GlobalValue *> &LocalsUsed) {
   // Summary not currently supported for anonymous functions, they should
   // have been named.
   assert(F.hasName());
@@ -89,6 +90,7 @@ static void computeFunctionSummary(ModuleSummaryIndex &Index, const Module &M,
   DenseMap<GlobalValue::GUID, CalleeInfo> IndirectCallEdges;
   DenseSet<const Value *> RefEdges;
   ICallPromotionAnalysis ICallAnalysis;
+  bool HasLocalsInUsed = !LocalsUsed.empty();
 
   SmallPtrSet<const User *, 8> Visited;
   for (const BasicBlock &BB : F)
@@ -100,6 +102,15 @@ static void computeFunctionSummary(ModuleSummaryIndex &Index, const Module &M,
       auto CS = ImmutableCallSite(&I);
       if (!CS)
         continue;
+
+      const auto *CI = dyn_cast<CallInst>(&I);
+      // Since we don't know exactly which local values are referenced in inline
+      // assembly, conservatively reference all of them from this function, to
+      // ensure we don't export a reference (which would require renaming and
+      // promotion).
+      if (HasLocalsInUsed && CI && CI->isInlineAsm())
+        RefEdges.insert(LocalsUsed.begin(), LocalsUsed.end());
+
       auto *CalledValue = CS.getCalledValue();
       auto *CalledFunction = CS.getCalledFunction();
       // Check if this is an alias to a function. If so, get the
@@ -127,7 +138,6 @@ static void computeFunctionSummary(ModuleSummaryIndex &Index, const Module &M,
                                    : CalleeInfo::HotnessType::Unknown;
         CallGraphEdges[CalleeId].updateHotness(Hotness);
       } else {
-        const auto *CI = dyn_cast<CallInst>(&I);
         // Skip inline assembly calls.
         if (CI && CI->isInlineAsm())
           continue;
@@ -183,11 +193,17 @@ ModuleSummaryIndex llvm::buildModuleSummaryIndex(
     std::function<BlockFrequencyInfo *(const Function &F)> 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;
+
+  // 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;
+  collectUsedGlobalVariables(M, Used, /*CompilerUsed*/ false);
+  SmallPtrSet<GlobalValue *, 8> LocalsUsed;
+  for (auto *V : Used) {
+    if (V->hasLocalLinkage())
+      LocalsUsed.insert(V);
+  }
 
   // Compute summaries for all functions defined in module, and save in the
   // index.
@@ -206,7 +222,7 @@ ModuleSummaryIndex llvm::buildModuleSummaryIndex(
       BFI = BFIPtr.get();
     }
 
-    computeFunctionSummary(Index, M, F, BFI, PSI);
+    computeFunctionSummary(Index, M, F, BFI, PSI, LocalsUsed);
   }
 
   // Compute summaries for all variables defined in module, and save in the
@@ -222,6 +238,12 @@ ModuleSummaryIndex llvm::buildModuleSummaryIndex(
   for (const GlobalAlias &A : M.aliases())
     computeAliasSummary(Index, A);
 
+  for (auto *V : LocalsUsed) {
+    auto *Summary = Index.getGlobalValueSummary(*V);
+    assert(Summary && "Missing summary for global value");
+    Summary->setNoRename();
+  }
+
   return Index;
 }
 
@@ -279,41 +301,3 @@ void ModuleSummaryIndexWrapperPass::getAnalysisUsage(AnalysisUsage &AU) const {
   AU.addRequired<BlockFrequencyInfoWrapperPass>();
   AU.addRequired<ProfileSummaryInfoWrapperPass>();
 }
-
-bool llvm::moduleCanBeRenamedForThinLTO(const Module &M) {
-  // We cannot currently promote or rename anything used in inline assembly,
-  // which are not visible to the compiler. Detect a possible case by looking
-  // for a llvm.used local value, in conjunction with an inline assembly call
-  // in the module. Prevent importing of any modules containing these uses by
-  // suppressing generation of the index. This also prevents importing
-  // 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.
-  // - Prevent importing of any functions with flag set.
-  // - Prevent importing of any global function with the same name as a
-  //   function in current module that has the flag set.
-  // - For any llvm.used value that is exported and promoted, add a private
-  //   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).
-  SmallPtrSet<GlobalValue *, 8> Used;
-  collectUsedGlobalVariables(M, Used, /*CompilerUsed*/ false);
-  bool LocalIsUsed =
-      any_of(Used, [](GlobalValue *V) { return V->hasLocalLinkage(); });
-  if (!LocalIsUsed)
-    return true;
-
-  // Walk all the instructions in the module and find if one is inline ASM
-  auto HasInlineAsm = any_of(M, [](const Function &F) {
-    return any_of(instructions(F), [](const Instruction &I) {
-      const CallInst *CallI = dyn_cast<CallInst>(&I);
-      if (!CallI)
-        return false;
-      return CallI->isInlineAsm();
-    });
-  });
-  return !HasInlineAsm;
-}

diff --git a/llvm/lib/Transforms/Utils/FunctionImportUtils.cpp b/llvm/lib/Transforms/Utils/FunctionImportUtils.cpp
index d14920575a4c732b2643852b55c4b2da90db5a53..b9b3606ecfb590a3e751fefef5417186387fdeb6 100644 (file)
--- a/llvm/lib/Transforms/Utils/FunctionImportUtils.cpp
+++ b/llvm/lib/Transforms/Utils/FunctionImportUtils.cpp
@@ -220,14 +220,6 @@ void FunctionImportGlobalProcessing::processGlobalForThinLTO(GlobalValue &GV) {
 }
 
 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)