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

Teresa Johnson via llvm-commits llvm-commits at lists.llvm.org
Sat Oct 29 22:40:45 PDT 2016


Author: tejohnson
Date: Sun Oct 30 00:40:44 2016
New Revision: 285513

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

Modified:
    llvm/trunk/include/llvm/Analysis/ModuleSummaryAnalysis.h
    llvm/trunk/include/llvm/IR/ModuleSummaryIndex.h
    llvm/trunk/lib/Analysis/ModuleSummaryAnalysis.cpp
    llvm/trunk/lib/Transforms/Utils/FunctionImportUtils.cpp

Modified: llvm/trunk/include/llvm/Analysis/ModuleSummaryAnalysis.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/Analysis/ModuleSummaryAnalysis.h?rev=285513&r1=285512&r2=285513&view=diff
==============================================================================
--- llvm/trunk/include/llvm/Analysis/ModuleSummaryAnalysis.h (original)
+++ llvm/trunk/include/llvm/Analysis/ModuleSummaryAnalysis.h Sun Oct 30 00:40:44 2016
@@ -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

Modified: llvm/trunk/include/llvm/IR/ModuleSummaryIndex.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/IR/ModuleSummaryIndex.h?rev=285513&r1=285512&r2=285513&view=diff
==============================================================================
--- llvm/trunk/include/llvm/IR/ModuleSummaryIndex.h (original)
+++ llvm/trunk/include/llvm/IR/ModuleSummaryIndex.h Sun Oct 30 00:40:44 2016
@@ -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); }

Modified: llvm/trunk/lib/Analysis/ModuleSummaryAnalysis.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Analysis/ModuleSummaryAnalysis.cpp?rev=285513&r1=285512&r2=285513&view=diff
==============================================================================
--- llvm/trunk/lib/Analysis/ModuleSummaryAnalysis.cpp (original)
+++ llvm/trunk/lib/Analysis/ModuleSummaryAnalysis.cpp Sun Oct 30 00:40:44 2016
@@ -77,7 +77,8 @@ static CalleeInfo::HotnessType getHotnes
 
 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(Modul
   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(Modul
       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(Modul
                                    : 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::buildModuleSumm
     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::buildModuleSumm
       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::buildModuleSumm
   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::getA
   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;
-}

Modified: llvm/trunk/lib/Transforms/Utils/FunctionImportUtils.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Utils/FunctionImportUtils.cpp?rev=285513&r1=285512&r2=285513&view=diff
==============================================================================
--- llvm/trunk/lib/Transforms/Utils/FunctionImportUtils.cpp (original)
+++ llvm/trunk/lib/Transforms/Utils/FunctionImportUtils.cpp Sun Oct 30 00:40:44 2016
@@ -220,14 +220,6 @@ void FunctionImportGlobalProcessing::pro
 }
 
 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)




More information about the llvm-commits mailing list