ThinLTO: fix assertion and refactor check for hidden use from inline ASM in a helper function

This test was crashing, and currently it breaks bootstrapping clang with debuginfo

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

From: Mehdi Amini <mehdi.amini@apple.com>
llvm-svn: 268715

5 files changed, 90 insertions, 54 deletions

diff --git a/llvm/include/llvm/Analysis/ModuleSummaryAnalysis.h b/llvm/include/llvm/Analysis/ModuleSummaryAnalysis.h
index 2d1c1630261..9f03610ba5b 100644
--- a/llvm/include/llvm/Analysis/ModuleSummaryAnalysis.h
+++ b/llvm/include/llvm/Analysis/ModuleSummaryAnalysis.h
@@ -81,6 +81,11 @@ 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/lib/Analysis/ModuleSummaryAnalysis.cpp b/llvm/lib/Analysis/ModuleSummaryAnalysis.cpp
index e33ec29a955..7c57c0592e6 100644
--- a/llvm/lib/Analysis/ModuleSummaryAnalysis.cpp
+++ b/llvm/lib/Analysis/ModuleSummaryAnalysis.cpp
@@ -120,37 +120,11 @@ ModuleSummaryIndexBuilder::ModuleSummaryIndexBuilder(
     const Module *M,
     std::function<BlockFrequencyInfo *(const Function &F)> Ftor)
     : Index(llvm::make_unique<ModuleSummaryIndex>()), M(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 = false;
-  for (GlobalValue *V : Used) {
-    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;
+  // 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;
 
   // Compute summaries for all functions defined in module, and save in the
   // index.
@@ -216,3 +190,41 @@ void ModuleSummaryIndexWrapperPass::getAnalysisUsage(AnalysisUsage &AU) const {
   AU.setPreservesAll();
   AU.addRequired<BlockFrequencyInfoWrapperPass>();
 }
+
+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 =
+      llvm::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 = llvm::any_of(M, [](const Function &F) {
+    return llvm::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 33ce5ccefda..92e7fbc7e69 100644
--- a/llvm/lib/Transforms/Utils/FunctionImportUtils.cpp
+++ b/llvm/lib/Transforms/Utils/FunctionImportUtils.cpp
@@ -12,6 +12,7 @@
 //
 //===----------------------------------------------------------------------===//
 
+#include "llvm/Analysis/ModuleSummaryAnalysis.h"
 #include "llvm/Transforms/Utils/FunctionImportUtils.h"
 #include "llvm/IR/InstIterator.h"
 #include "llvm/IR/Instructions.h"
@@ -213,31 +214,13 @@ void FunctionImportGlobalProcessing::processGlobalForThinLTO(GlobalValue &GV) {
 }
 
 void FunctionImportGlobalProcessing::processGlobalsForThinLTO() {
-  // 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 (!moduleCanBeRenamedForThinLTO(M)) {
     // We would have blocked importing from this module by suppressing index
-    // generation.
-    assert((!V->hasLocalLinkage() || !isPerformingImport()) &&
-           "Should have blocked importing from module with local used");
-    if ((LocalIsUsed |= V->hasLocalLinkage()))
-      break;
+    // 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;
   }
-  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);
diff --git a/llvm/test/ThinLTO/X86/Inputs/llvm.used.ll b/llvm/test/ThinLTO/X86/Inputs/llvm.used.ll
new file mode 100644
index 00000000000..35f5529c2b6
--- /dev/null
+++ b/llvm/test/ThinLTO/X86/Inputs/llvm.used.ll
@@ -0,0 +1,11 @@
+target datalayout = "e-m:o-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-apple-macosx10.11.0"
+
+
+define i32 @main() #0 {
+entry:
+  call void (...) @globalfunc()
+  ret i32 0
+}
+
+declare void @globalfunc(...)
\ No newline at end of file
diff --git a/llvm/test/ThinLTO/X86/llvm.used.ll b/llvm/test/ThinLTO/X86/llvm.used.ll
new file mode 100644
index 00000000000..4b379f72c25
--- /dev/null
+++ b/llvm/test/ThinLTO/X86/llvm.used.ll
@@ -0,0 +1,25 @@
+; Do setup work for all below tests: generate bitcode and combined index
+; RUN: opt -module-summary %s -o %t.bc
+; RUN: opt -module-summary %p/Inputs/llvm.used.ll -o %t2.bc
+; RUN: llvm-lto -thinlto-action=thinlink -o %t3.bc %t.bc %t2.bc
+
+
+; RUN: llvm-lto -thinlto-action=import %t2.bc -thinlto-index=%t3.bc -o - | llvm-dis -o - | FileCheck %s
+; CHECK: define available_externally void @globalfunc
+
+
+target datalayout = "e-m:o-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-apple-macosx10.11.0"
+
+
+
+define internal void @_ZN12_GLOBAL__N_16Module4dumpEv() {
+    ret void
+}
+@llvm.used = appending global [1 x i8*] [i8* bitcast (void ()* @_ZN12_GLOBAL__N_16Module4dumpEv to i8*)], section "llvm.metadata"
+
+
+define void @globalfunc() #0 {
+entry:
+  ret void
+}