[PATCH] D26250: [ThinLTO] Handle distributed backend case when doing renaming

[Teresa Johnson via llvm-commits]

tejohnson created this revision.
tejohnson added a reviewer: mehdi_amini.
tejohnson added a subscriber: llvm-commits.

The recent change I made to consult the summary when deciding whether to
rename (to handle inline asm) in r285513 broke the distributed build
case. In a distributed backend we will only have a portion of the
combined index, specifically for imported modules we only have the
summaries for any imported definitions. When renaming on import we were
asserting because no summary entry was found for a local reference being
linked in (def wasn't imported).

We only need to consult the summary for a renaming decision for the
exporting module. For imports, we would have prevented importing any
references to NoRename values already. Handle the missing summary,
asserting that we are not exporting in that case.


https://reviews.llvm.org/D26250

Files:
  lib/Transforms/Utils/FunctionImportUtils.cpp
  test/ThinLTO/X86/Inputs/distributed_import.ll
  test/ThinLTO/X86/distributed_import.ll


Index: test/ThinLTO/X86/distributed_import.ll
===================================================================
--- /dev/null
+++ test/ThinLTO/X86/distributed_import.ll
@@ -0,0 +1,20 @@
+; RUN: opt -module-summary %s -o %t1.bc
+; RUN: opt -module-summary %p/Inputs/distributed_import.ll -o %t2.bc
+
+; RUN: llvm-lto2 %t1.bc %t2.bc -o %t.o -save-temps \
+; RUN:     -thinlto-distributed-indexes \
+; RUN:     -r=%t1.bc,g, \
+; RUN:     -r=%t1.bc,f,px \
+; RUN:     -r=%t2.bc,g,px
+; RUN:  opt -function-import -summary-file %t1.bc.thinlto.bc %t1.bc -o %t1.out
+; RUN: opt -function-import -summary-file %t2.bc.thinlto.bc %t2.bc -o %t2.out
+; RUN: llvm-dis -o - %t2.out | FileCheck %s
+; CHECK: @G.llvm.0
+
+declare i32 @g(...)
+
+define void @f() {
+entry:
+  call i32 (...) @g()
+  ret void
+}
Index: test/ThinLTO/X86/Inputs/distributed_import.ll
===================================================================
--- /dev/null
+++ test/ThinLTO/X86/Inputs/distributed_import.ll
@@ -0,0 +1,6 @@
+ at G = internal global i32 7
+define i32 @g() {
+entry:
+  %0 = load i32, i32* @G
+  ret i32 %0
+}
Index: lib/Transforms/Utils/FunctionImportUtils.cpp
===================================================================
--- lib/Transforms/Utils/FunctionImportUtils.cpp
+++ lib/Transforms/Utils/FunctionImportUtils.cpp
@@ -67,10 +67,24 @@
   if (GVar && GVar->isConstant() && GVar->hasGlobalUnnamedAddr())
     return false;
 
-  auto *Summary = ImportIndex.getGlobalValueSummary(SGV->getGUID());
-  assert(Summary && "Missing summary for global value");
-  if (Summary->noRename())
-    return false;
+  // If we are exporting, we need to see whether this value is marked
+  // as NoRename in the summary. If we are importing, we may not have
+  // a summary in the distributed backend case (only summaries for values
+  // importes as defs, not references, are included in the index passed
+  // to the distributed backends).
+  auto Summaries = ImportIndex.findGlobalValueSummaryList(SGV->getGUID());
+  if (Summaries == ImportIndex.end())
+    // Assert that this is an import - we should always have access to the
+    // summary when exporting.
+    assert(isPerformingImport() &&
+           "Missing summary for global value when exporting");
+  else {
+    assert(Summaries->second.size() == 1 && "Local has more than one summary");
+    if (Summaries->second.front()->noRename()) {
+      assert(isModuleExporting() && "Imported a non-renamable local value");
+      return false;
+    }
+  }
 
   // Eventually we only need to promote functions in the exporting module that
   // are referenced by a potentially exported function (i.e. one that is in the


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D26250.76733.patch
Type: text/x-patch
Size: 2660 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20161102/2a1c42a0/attachment.bin>