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

[Teresa Johnson via llvm-commits]

tejohnson updated this revision to Diff 76746.
tejohnson added a comment.

The previous assert was too strict - if it is NoRename and we are importing
we should only assert if it was marked as a global to import.


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,25 @@
   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() || !GlobalsToImport->count(SGV)) &&
+             "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.76746.patch
Type: text/x-patch
Size: 2708 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20161102/e7f0aea6/attachment.bin>