[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

1 files changed, 30 insertions, 46 deletions

diff --git a/llvm/lib/Analysis/ModuleSummaryAnalysis.cpp b/llvm/lib/Analysis/ModuleSummaryAnalysis.cpp
index f13ae844442..09d18eb3da1 100644
--- 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;
-}