[llvm] r267717 - [ThinLTO] Refine fix to avoid renaming of uses in inline assembly.

[Teresa Johnson via llvm-commits]

Author: tejohnson
Date: Wed Apr 27 09:19:38 2016
New Revision: 267717

URL: http://llvm.org/viewvc/llvm-project?rev=267717&view=rev
Log:
[ThinLTO] Refine fix to avoid renaming of uses in inline assembly.

Summary:
Refine the workaround from r266877 that attempts to prevent
renaming of locals in inline assembly, so that in addition to looking
for a llvm.used local value, that there is at least one inline assembly
call in the module. Otherwise, debug functions added to the llvm.used
can block importing/exporting unnecessarily.

Reviewers: joker.eph

Subscribers: llvm-commits, joker.eph

Differential Revision: http://reviews.llvm.org/D19573

Modified:
    llvm/trunk/lib/Analysis/ModuleSummaryAnalysis.cpp
    llvm/trunk/lib/Transforms/Utils/FunctionImportUtils.cpp

Modified: llvm/trunk/lib/Analysis/ModuleSummaryAnalysis.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Analysis/ModuleSummaryAnalysis.cpp?rev=267717&r1=267716&r2=267717&view=diff
==============================================================================
--- llvm/trunk/lib/Analysis/ModuleSummaryAnalysis.cpp (original)
+++ llvm/trunk/lib/Analysis/ModuleSummaryAnalysis.cpp Wed Apr 27 09:19:38 2016
@@ -19,6 +19,7 @@
 #include "llvm/Analysis/LoopInfo.h"
 #include "llvm/IR/CallSite.h"
 #include "llvm/IR/Dominators.h"
+#include "llvm/IR/InstIterator.h"
 #include "llvm/IR/IntrinsicInst.h"
 #include "llvm/IR/ValueSymbolTable.h"
 #include "llvm/Pass.h"
@@ -119,10 +120,10 @@ ModuleSummaryIndexBuilder::ModuleSummary
     const Module *M,
     std::function<BlockFrequencyInfo *(const Function &F)> Ftor)
     : Index(llvm::make_unique<ModuleSummaryIndex>()), M(M) {
-  // We cannot currently promote or rename anything that is in llvm.used,
-  // since any such value may have a use that won't see the new name.
-  // Specifically, any uses within inline assembly are not visible to the
-  // compiler. Prevent importing of any modules containing these uses by
+  // 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
@@ -139,10 +140,17 @@ ModuleSummaryIndexBuilder::ModuleSummary
   //   with a reference could be exported).
   SmallPtrSet<GlobalValue *, 8> Used;
   collectUsedGlobalVariables(*M, Used, /*CompilerUsed*/ false);
+  bool LocalIsUsed = false;
   for (GlobalValue *V : Used) {
-    if (V->hasLocalLinkage())
-      return;
+    if ((LocalIsUsed |= V->hasLocalLinkage()))
+      break;
   }
+  if (LocalIsUsed)
+    for (auto &F : *M)
+      for (auto &I : instructions(F))
+        if (const CallInst *CallI = dyn_cast<CallInst>(&I))
+          if (CallI->isInlineAsm())
+            return;
 
   // Compute summaries for all functions defined in module, and save in the
   // index.

Modified: llvm/trunk/lib/Transforms/Utils/FunctionImportUtils.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Utils/FunctionImportUtils.cpp?rev=267717&r1=267716&r2=267717&view=diff
==============================================================================
--- llvm/trunk/lib/Transforms/Utils/FunctionImportUtils.cpp (original)
+++ llvm/trunk/lib/Transforms/Utils/FunctionImportUtils.cpp Wed Apr 27 09:19:38 2016
@@ -13,6 +13,8 @@
 //===----------------------------------------------------------------------===//
 
 #include "llvm/Transforms/Utils/FunctionImportUtils.h"
+#include "llvm/IR/InstIterator.h"
+#include "llvm/IR/Instructions.h"
 using namespace llvm;
 
 /// Checks if we should import SGV as a definition, otherwise import as a
@@ -211,25 +213,31 @@ void FunctionImportGlobalProcessing::pro
 }
 
 void FunctionImportGlobalProcessing::processGlobalsForThinLTO() {
-  // We cannot currently promote or rename anything that is in llvm.used,
-  // since any such value may have a use that won't see the new name.
-  // Specifically, any uses within inline assembly are not visible to the
-  // compiler. Prevent changing any such values on the exporting side,
+  // 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 changing any such values on the exporting side,
   // since we would already have guarded against an import from this module by
   // suppressing its index generation. See comments on what is required
   // in order to implement a finer grained solution in
   // ModuleSummaryIndexBuilder::ModuleSummaryIndexBuilder().
   SmallPtrSet<GlobalValue *, 8> Used;
   collectUsedGlobalVariables(M, Used, /*CompilerUsed*/ false);
+  bool LocalIsUsed = false;
   for (GlobalValue *V : Used) {
-    if (!V->hasLocalLinkage())
-      continue;
     // We would have blocked importing from this module by suppressing index
     // generation.
-    assert(!isPerformingImport() &&
+    assert((!V->hasLocalLinkage() || !isPerformingImport()) &&
            "Should have blocked importing from module with local used");
-    return;
+    if ((LocalIsUsed |= V->hasLocalLinkage()))
+      break;
   }
+  if (LocalIsUsed)
+    for (auto &F : M)
+      for (auto &I : instructions(F))
+        if (const CallInst *CallI = dyn_cast<CallInst>(&I))
+          if (CallI->isInlineAsm())
+            return;
 
   for (GlobalVariable &GV : M.globals())
     processGlobalForThinLTO(GV);