[llvm] [ThinLTO] Do not duplicate import a function that is actually defined in the current module #110064 (PR #111933)

William Junda Huang via llvm-commits llvm-commits at lists.llvm.org
Thu Oct 24 17:12:00 PDT 2024


https://github.com/huangjd updated https://github.com/llvm/llvm-project/pull/111933

>From a00ac362a6d0d2b2247c841331f2cd93c6af5114 Mon Sep 17 00:00:00 2001
From: William Huang <williamjhuang at google.com>
Date: Wed, 25 Sep 2024 17:50:20 -0400
Subject: [PATCH 1/6] [ThinLTO] Do not duplicate import a function that is
 actually defined in the current module.

Doing so could cause a bug where the linker tries to remap a function
"reimported" from the current module when materializing it, causing a
lookup assert in the type mappings.
---
 llvm/lib/Linker/IRMover.cpp | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/llvm/lib/Linker/IRMover.cpp b/llvm/lib/Linker/IRMover.cpp
index 3a6c2678cd157f..5bd05d86a949c3 100644
--- a/llvm/lib/Linker/IRMover.cpp
+++ b/llvm/lib/Linker/IRMover.cpp
@@ -595,11 +595,15 @@ Value *IRLinker::materialize(Value *V, bool ForIndirectSymbol) {
   if (!SGV)
     return nullptr;
 
+  // If SGV is from dest, it is already materialized when dest was loaded.
+  if (SGV->getParent() == &DstM)
+    return nullptr;
+
   // When linking a global from other modules than source & dest, skip
   // materializing it because it would be mapped later when its containing
   // module is linked. Linking it now would potentially pull in many types that
   // may not be mapped properly.
-  if (SGV->getParent() != &DstM && SGV->getParent() != SrcM.get())
+  if (SGV->getParent() != SrcM.get())
     return nullptr;
 
   Expected<Constant *> NewProto = linkGlobalValueProto(SGV, ForIndirectSymbol);

>From c27f83214c9f32293e78ca83dfd75705169a22bc Mon Sep 17 00:00:00 2001
From: William Huang <williamjhuang at google.com>
Date: Fri, 4 Oct 2024 00:16:36 -0400
Subject: [PATCH 2/6] added test case

---
 llvm/lib/Linker/IRMover.cpp                   |  2 +-
 .../Inputs/ditemplatevalueparameter-remap.ll  | 29 +++++++
 .../X86/ditemplatevalueparameter-remap.ll     | 87 +++++++++++++++++++
 3 files changed, 117 insertions(+), 1 deletion(-)
 create mode 100644 llvm/test/ThinLTO/X86/Inputs/ditemplatevalueparameter-remap.ll
 create mode 100644 llvm/test/ThinLTO/X86/ditemplatevalueparameter-remap.ll

diff --git a/llvm/lib/Linker/IRMover.cpp b/llvm/lib/Linker/IRMover.cpp
index 5bd05d86a949c3..5067fbff2e277b 100644
--- a/llvm/lib/Linker/IRMover.cpp
+++ b/llvm/lib/Linker/IRMover.cpp
@@ -595,7 +595,7 @@ Value *IRLinker::materialize(Value *V, bool ForIndirectSymbol) {
   if (!SGV)
     return nullptr;
 
-  // If SGV is from dest, it is already materialized when dest was loaded.
+  // If SGV is from dest, it was already materialized when dest was loaded.
   if (SGV->getParent() == &DstM)
     return nullptr;
 
diff --git a/llvm/test/ThinLTO/X86/Inputs/ditemplatevalueparameter-remap.ll b/llvm/test/ThinLTO/X86/Inputs/ditemplatevalueparameter-remap.ll
new file mode 100644
index 00000000000000..be93160b943397
--- /dev/null
+++ b/llvm/test/ThinLTO/X86/Inputs/ditemplatevalueparameter-remap.ll
@@ -0,0 +1,29 @@
+target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-i128:128-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+define void @_Z8thinlto1v() unnamed_addr {
+  %3 = alloca i64, align 4
+    #dbg_declare(ptr %3, !14, !DIExpression(), !15)
+  ret void
+}
+
+!llvm.dbg.cu = !{!0}
+!llvm.module.flags = !{!2, !3, !4, !5}
+
+!0 = distinct !DICompileUnit(language: DW_LANG_C_plus_plus_14, file: !1, producer: "clang", isOptimized: true, runtimeVersion: 0, emissionKind: FullDebug, splitDebugInlining: false, nameTableKind: None)
+!1 = !DIFile(filename: "B.cpp", directory: ".")
+!2 = !{i32 7, !"Dwarf Version", i32 4}
+!3 = !{i32 2, !"Debug Info Version", i32 3}
+!4 = !{i32 1, !"wchar_size", i32 4}
+!5 = !{i32 8, !"PIC Level", i32 2}
+!10 = distinct !DISubprogram(name: "thinlto1", linkageName: "_Z8thinlto1v", scope: !11, file: !11, line: 8, type: !12, scopeLine: 8, flags: DIFlagPrototyped | DIFlagAllCallsDescribed, spFlags: DISPFlagDefinition | DISPFlagOptimized, unit: !0)
+!11 = !DIFile(filename: "b.cpp", directory: ".")
+!12 = !DISubroutineType(types: !13)
+!13 = !{null}
+!14 = !DILocalVariable(name: "a", arg: 1, scope: !10, file: !11, line: 18, type: !16)
+!15 = !DILocation(line: 18, column: 19, scope: !10)
+!16 = distinct !DICompositeType(tag: DW_TAG_structure_type, name: "S<&func1>", file: !11, line: 2, size: 8, flags: DIFlagTypePassByValue, elements: !17, templateParams: !18, identifier: "_ZTS1SIXadL_Z5func1vEEE")
+!17 = !{}
+!18 = !{!19}
+!19 = !DITemplateValueParameter(name: "Func", type: !20, value: ptr undef)
+!20 = !DIDerivedType(tag: DW_TAG_pointer_type, baseType: !12, size: 64)
diff --git a/llvm/test/ThinLTO/X86/ditemplatevalueparameter-remap.ll b/llvm/test/ThinLTO/X86/ditemplatevalueparameter-remap.ll
new file mode 100644
index 00000000000000..0651705ccba8b8
--- /dev/null
+++ b/llvm/test/ThinLTO/X86/ditemplatevalueparameter-remap.ll
@@ -0,0 +1,87 @@
+; https://github.com/llvm/llvm-project/pull/110064
+; This test case checks if thinLTO correctly links metadata values in a specific
+; situation. Assume we are linking module B into module A, where an extern
+; function used in A is defined in B, but the function body has a
+; DITemplateValueParameter referring to another function back in A. The
+; compiler must check this other function is actually coming from A, thus
+; already materialized and does not require remapping. The IR here is modified
+; from the following source code.
+;
+; // A.h
+; template <void (*Func)()>
+; struct S {
+;   void Impl() {
+;     Func();
+;   }
+; };
+;
+; void func1();
+;
+; // A.cpp
+; #include "A.h"
+; __attribute__((weak)) void func1() {}
+; extern void thinlto1();
+; void bar() {
+;   S<func1> s; // Force instantiation of S<func1> in this compilation unit.
+;   s.Impl();
+;   thinlto1();
+; }
+;
+; // B.cpp
+; #include "A.h"
+; void thinlto1() {
+;   S<func1> s;
+; }
+;
+; RUN: opt -module-summary -o %t1.bc %s
+; RUN: opt -module-summary -o %t2.bc %S/Inputs/ditemplatevalueparameter-remap.ll
+; RUN: ld.lld --plugin-opt=thinlto-index-only -shared %t1.bc %t2.bc
+; RUN: clang -O3 -fthinlto-index=%t1.bc.thinlto.bc -x ir %t1.bc -S -emit-llvm -o - | FileCheck %s
+
+target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-i128:128-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+$_Z5func1v = comdat any
+
+define linkonce_odr dso_local void @_Z5func1v() unnamed_addr !dbg !10 {
+  ret void
+}
+
+; Dummy function to use _Z5func1v so that it is not treated as dead symbol.
+define void @_Z3bazv() {
+  tail call void @_Z5func1v()
+  ret void
+}
+
+declare void @_Z8thinlto1v() unnamed_addr
+
+; CHECK: void @_Z3barv()
+; CHECK-NOT: call void @_Z8thinlto1v()
+; CHECK-NEXT: ret void
+define void @_Z3barv() unnamed_addr !dbg !14 {
+  tail call void @_Z8thinlto1v(), !dbg !25
+  ret void
+}
+
+!llvm.dbg.cu = !{!0}
+!llvm.module.flags = !{!2, !3, !4, !5}
+
+!0 = distinct !DICompileUnit(language: DW_LANG_C_plus_plus_14, file: !1, producer: "clang", isOptimized: true, runtimeVersion: 0, emissionKind: FullDebug, splitDebugInlining: false, nameTableKind: None)
+!1 = !DIFile(filename: "A.cpp", directory: ".")
+!2 = !{i32 7, !"Dwarf Version", i32 4}
+!3 = !{i32 2, !"Debug Info Version", i32 3}
+!4 = !{i32 1, !"wchar_size", i32 4}
+!5 = !{i32 8, !"PIC Level", i32 2}
+!10 = distinct !DISubprogram(name: "func1", linkageName: "_Z5func1v", scope: !11, file: !11, line: 6, type: !12, scopeLine: 6, flags: DIFlagPrototyped | DIFlagAllCallsDescribed, spFlags: DISPFlagDefinition | DISPFlagOptimized, unit: !0)
+!11 = !DIFile(filename: "a.h", directory: ".")
+!12 = !DISubroutineType(types: !13)
+!13 = !{null}
+!14 = distinct !DISubprogram(name: "bar", linkageName: "_Z3barv", scope: !11, file: !11, line: 15, type: !12, scopeLine: 15, flags: DIFlagPrototyped | DIFlagAllCallsDescribed, spFlags: DISPFlagDefinition | DISPFlagOptimized, unit: !0, retainedNodes: !16)
+!16 = !{!17}
+!17 = !DILocalVariable(name: "s", scope: !14, file: !11, line: 10, type: !18)
+!18 = distinct !DICompositeType(tag: DW_TAG_structure_type, name: "S<&func1>", file: !11, line: 2, size: 8, flags: DIFlagTypePassByValue, elements: !19, templateParams: !20, identifier: "_ZTS1SIXadL_Z5func1vEEE")
+!19 = !{}
+!20 = !{!21}
+!21 = !DITemplateValueParameter(name: "Func", type: !22, value: ptr @_Z5func1v)
+!22 = !DIDerivedType(tag: DW_TAG_pointer_type, baseType: !12, size: 64)
+!25 = !DILocation(line: 16, column: 5, scope: !14)

>From 8cb1d36954c5f8d575d8def587cf6fd03de62a98 Mon Sep 17 00:00:00 2001
From: William Huang <williamjhuang at google.com>
Date: Thu, 10 Oct 2024 22:19:53 -0400
Subject: [PATCH 3/6] update test case

---
 llvm/test/ThinLTO/X86/ditemplatevalueparameter-remap.ll | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/llvm/test/ThinLTO/X86/ditemplatevalueparameter-remap.ll b/llvm/test/ThinLTO/X86/ditemplatevalueparameter-remap.ll
index 0651705ccba8b8..76b4b04575bc8c 100644
--- a/llvm/test/ThinLTO/X86/ditemplatevalueparameter-remap.ll
+++ b/llvm/test/ThinLTO/X86/ditemplatevalueparameter-remap.ll
@@ -35,8 +35,13 @@
 ;
 ; RUN: opt -module-summary -o %t1.bc %s
 ; RUN: opt -module-summary -o %t2.bc %S/Inputs/ditemplatevalueparameter-remap.ll
-; RUN: ld.lld --plugin-opt=thinlto-index-only -shared %t1.bc %t2.bc
-; RUN: clang -O3 -fthinlto-index=%t1.bc.thinlto.bc -x ir %t1.bc -S -emit-llvm -o - | FileCheck %s
+; RUN: llvm-lto2 run %t1.bc %t2.bc -o %t3.o -save-temps \
+; RUN:   -r=%t1.bc,_Z5func1v,p    \
+; RUN:   -r=%t1.bc,_Z3bazv,       \
+; RUN:   -r=%t1.bc,_Z8thinlto1v,  \
+; RUN:   -r=%t1.bc,_Z3barv,px     \
+; RUN:   -r=%t2.bc,_Z8thinlto1v,p
+; RUN: clang -O3 -fthinlto-index=%t3.o.index.bc -x ir %t1.bc -S -emit-llvm -o - | FileCheck %s
 
 target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-i128:128-f80:128-n8:16:32:64-S128"
 target triple = "x86_64-unknown-linux-gnu"

>From 12ed3c89cb28cb41ef6156e19abcb7ca0c496d2a Mon Sep 17 00:00:00 2001
From: William Huang <williamjhuang at google.com>
Date: Wed, 23 Oct 2024 15:54:39 -0400
Subject: [PATCH 4/6] Also fix a related issue with internalize pass where it
 crashes thinLTO if opt applies internalize before thinLTO import function
 even if the same internalize decision is applied. This issue appears to be a
 cofactor of the original issue being fixed in this patch, as the updated test
 case crashes the compiler without patching either location.

---
 .../Transforms/Utils/FunctionImportUtils.cpp  | 13 ++++++++++++
 .../X86/ditemplatevalueparameter-remap.ll     | 20 +++++++++++--------
 2 files changed, 25 insertions(+), 8 deletions(-)

diff --git a/llvm/lib/Transforms/Utils/FunctionImportUtils.cpp b/llvm/lib/Transforms/Utils/FunctionImportUtils.cpp
index 766c7501550da5..bbc6222723dd2a 100644
--- a/llvm/lib/Transforms/Utils/FunctionImportUtils.cpp
+++ b/llvm/lib/Transforms/Utils/FunctionImportUtils.cpp
@@ -223,6 +223,19 @@ void FunctionImportGlobalProcessing::processGlobalForThinLTO(GlobalValue &GV) {
   if (GV.hasName())
     VI = ImportIndex.getValueInfo(GV.getGUID());
 
+  // Try lookup ValueInfo again without local symbol prefix, to handle a case
+  // where a GV with internal linkage somehow show up in ThinLTO index. We need
+  // to check if the GV in ThinLTO index is actually coming from the current
+  // module, in this case it is considered a match, as if this GV is being
+  // imported from this module itself.
+  if (!VI && GV.getLinkage() == GlobalValue::InternalLinkage)
+    if (ValueInfo VI2 = ImportIndex.getValueInfo(
+            GlobalValue::getGUID(GlobalValue::getGlobalIdentifier(
+                GV.getName(), GlobalValue::ExternalLinkage, ""))))
+      if (!VI2.getSummaryList().empty() &&
+          VI2.getSummaryList()[0]->modulePath() == GV.getParent()->getName())
+        return;
+
   // We should always have a ValueInfo (i.e. GV in index) for definitions when
   // we are exporting, and also when importing that value.
   assert(VI || GV.isDeclaration() ||
diff --git a/llvm/test/ThinLTO/X86/ditemplatevalueparameter-remap.ll b/llvm/test/ThinLTO/X86/ditemplatevalueparameter-remap.ll
index 76b4b04575bc8c..238d2cfa21e943 100644
--- a/llvm/test/ThinLTO/X86/ditemplatevalueparameter-remap.ll
+++ b/llvm/test/ThinLTO/X86/ditemplatevalueparameter-remap.ll
@@ -35,20 +35,23 @@
 ;
 ; RUN: opt -module-summary -o %t1.bc %s
 ; RUN: opt -module-summary -o %t2.bc %S/Inputs/ditemplatevalueparameter-remap.ll
-; RUN: llvm-lto2 run %t1.bc %t2.bc -o %t3.o -save-temps \
+; RUN: llvm-lto2 run --thinlto-distributed-indexes %t1.bc %t2.bc -o %t3.o -save-temps \
 ; RUN:   -r=%t1.bc,_Z5func1v,p    \
-; RUN:   -r=%t1.bc,_Z3bazv,       \
-; RUN:   -r=%t1.bc,_Z8thinlto1v,  \
+; RUN:   -r=%t1.bc,_Z3bazv,px     \
+; RUN:   -r=%t1.bc,_Z8thinlto1v,x \
 ; RUN:   -r=%t1.bc,_Z3barv,px     \
-; RUN:   -r=%t2.bc,_Z8thinlto1v,p
-; RUN: clang -O3 -fthinlto-index=%t3.o.index.bc -x ir %t1.bc -S -emit-llvm -o - | FileCheck %s
+; RUN:   -r=%t2.bc,_Z8thinlto1v,px
+; RUN: opt -passes='internalize<preserve-gv=_Z3bazv;preserve-gv=_Z3barv>,function-import,module-inline' \
+; RUN:   -enable-import-metadata -import-all-index \
+; RUN:   -summary-file=%t1.bc.thinlto.bc %t1.bc -o %t1.o
+; RUN: llvm-dis %t1.o -o - | FileCheck %s
 
 target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-i128:128-f80:128-n8:16:32:64-S128"
 target triple = "x86_64-unknown-linux-gnu"
 
 $_Z5func1v = comdat any
 
-define linkonce_odr dso_local void @_Z5func1v() unnamed_addr !dbg !10 {
+define linkonce_odr void @_Z5func1v() unnamed_addr !dbg !10 {
   ret void
 }
 
@@ -60,9 +63,10 @@ define void @_Z3bazv() {
 
 declare void @_Z8thinlto1v() unnamed_addr
 
+; Check _Z8thinlto1v is inlined after thinLTO.
 ; CHECK: void @_Z3barv()
-; CHECK-NOT: call void @_Z8thinlto1v()
-; CHECK-NEXT: ret void
+; CHECK-NOT: @_Z8thinlto1v()
+; CHECK: ret void
 define void @_Z3barv() unnamed_addr !dbg !14 {
   tail call void @_Z8thinlto1v(), !dbg !25
   ret void

>From 69d02dabd8b74f952bba4c5ee669e71f4f4e7f68 Mon Sep 17 00:00:00 2001
From: William Huang <williamjhuang at google.com>
Date: Thu, 24 Oct 2024 05:57:53 -0400
Subject: [PATCH 5/6] fix thinLTO internalize when being called from opt

---
 llvm/lib/Transforms/IPO/FunctionImport.cpp    | 25 +++++++++++--------
 .../Transforms/Utils/FunctionImportUtils.cpp  | 13 ----------
 .../X86/ditemplatevalueparameter-remap.ll     |  5 ++--
 3 files changed, 16 insertions(+), 27 deletions(-)

diff --git a/llvm/lib/Transforms/IPO/FunctionImport.cpp b/llvm/lib/Transforms/IPO/FunctionImport.cpp
index 1aac8e07135875..b4cefda70d75c3 100644
--- a/llvm/lib/Transforms/IPO/FunctionImport.cpp
+++ b/llvm/lib/Transforms/IPO/FunctionImport.cpp
@@ -2006,17 +2006,6 @@ static bool doImportingForModuleForTest(
     ComputeCrossModuleImportForModuleForTest(M.getModuleIdentifier(),
                                              isPrevailing, *Index, ImportList);
 
-  // Conservatively mark all internal values as promoted. This interface is
-  // only used when doing importing via the function importing pass. The pass
-  // is only enabled when testing importing via the 'opt' tool, which does
-  // not do the ThinLink that would normally determine what values to promote.
-  for (auto &I : *Index) {
-    for (auto &S : I.second.SummaryList) {
-      if (GlobalValue::isLocalLinkage(S->linkage()))
-        S->setLinkage(GlobalValue::ExternalLinkage);
-    }
-  }
-
   // Next we need to promote to global scope and rename any local values that
   // are potentially exported to other modules.
   if (renameModuleForThinLTO(M, *Index, /*ClearDSOLocalOnDeclarations=*/false,
@@ -2025,6 +2014,20 @@ static bool doImportingForModuleForTest(
     return true;
   }
 
+  // Perform internalization of GVs in this module, if they are marked as
+  // internal by thinLTO summary.
+  GVSummaryMapTy DefinedGlobals;
+  for (const auto &GlobalList : *Index) {
+    auto GUID = GlobalList.first;
+    for (const auto &Summary : GlobalList.second.SummaryList) {
+      if (Summary->modulePath() == M.getName()) {
+        DefinedGlobals[GUID] = Summary.get();
+      }
+    }
+  }
+  thinLTOFinalizeInModule(M, DefinedGlobals, /*PropagateAttrs=*/true);
+  thinLTOInternalizeModule(M, DefinedGlobals);
+
   // Perform the import now.
   auto ModuleLoader = [&M](StringRef Identifier) {
     return loadFile(std::string(Identifier), M.getContext());
diff --git a/llvm/lib/Transforms/Utils/FunctionImportUtils.cpp b/llvm/lib/Transforms/Utils/FunctionImportUtils.cpp
index bbc6222723dd2a..766c7501550da5 100644
--- a/llvm/lib/Transforms/Utils/FunctionImportUtils.cpp
+++ b/llvm/lib/Transforms/Utils/FunctionImportUtils.cpp
@@ -223,19 +223,6 @@ void FunctionImportGlobalProcessing::processGlobalForThinLTO(GlobalValue &GV) {
   if (GV.hasName())
     VI = ImportIndex.getValueInfo(GV.getGUID());
 
-  // Try lookup ValueInfo again without local symbol prefix, to handle a case
-  // where a GV with internal linkage somehow show up in ThinLTO index. We need
-  // to check if the GV in ThinLTO index is actually coming from the current
-  // module, in this case it is considered a match, as if this GV is being
-  // imported from this module itself.
-  if (!VI && GV.getLinkage() == GlobalValue::InternalLinkage)
-    if (ValueInfo VI2 = ImportIndex.getValueInfo(
-            GlobalValue::getGUID(GlobalValue::getGlobalIdentifier(
-                GV.getName(), GlobalValue::ExternalLinkage, ""))))
-      if (!VI2.getSummaryList().empty() &&
-          VI2.getSummaryList()[0]->modulePath() == GV.getParent()->getName())
-        return;
-
   // We should always have a ValueInfo (i.e. GV in index) for definitions when
   // we are exporting, and also when importing that value.
   assert(VI || GV.isDeclaration() ||
diff --git a/llvm/test/ThinLTO/X86/ditemplatevalueparameter-remap.ll b/llvm/test/ThinLTO/X86/ditemplatevalueparameter-remap.ll
index 238d2cfa21e943..c462232b90cfd6 100644
--- a/llvm/test/ThinLTO/X86/ditemplatevalueparameter-remap.ll
+++ b/llvm/test/ThinLTO/X86/ditemplatevalueparameter-remap.ll
@@ -41,9 +41,8 @@
 ; RUN:   -r=%t1.bc,_Z8thinlto1v,x \
 ; RUN:   -r=%t1.bc,_Z3barv,px     \
 ; RUN:   -r=%t2.bc,_Z8thinlto1v,px
-; RUN: opt -passes='internalize<preserve-gv=_Z3bazv;preserve-gv=_Z3barv>,function-import,module-inline' \
-; RUN:   -enable-import-metadata -import-all-index \
-; RUN:   -summary-file=%t1.bc.thinlto.bc %t1.bc -o %t1.o
+; RUN: opt -passes='function-import,module-inline' -enable-import-metadata \
+; RUN:   -import-all-index -summary-file=%t1.bc.thinlto.bc %t1.bc -o %t1.o
 ; RUN: llvm-dis %t1.o -o - | FileCheck %s
 
 target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-i128:128-f80:128-n8:16:32:64-S128"

>From d1202bdfc74940bb2854333e8832f5381ea782c1 Mon Sep 17 00:00:00 2001
From: William Huang <williamjhuang at google.com>
Date: Thu, 24 Oct 2024 20:11:46 -0400
Subject: [PATCH 6/6] reverted changes in FunctionImportUtils, use
 non-distributed thinLTO instead

---
 llvm/lib/Transforms/IPO/FunctionImport.cpp    | 25 ++++++++-----------
 .../X86/ditemplatevalueparameter-remap.ll     |  8 +++---
 2 files changed, 14 insertions(+), 19 deletions(-)

diff --git a/llvm/lib/Transforms/IPO/FunctionImport.cpp b/llvm/lib/Transforms/IPO/FunctionImport.cpp
index b4cefda70d75c3..1aac8e07135875 100644
--- a/llvm/lib/Transforms/IPO/FunctionImport.cpp
+++ b/llvm/lib/Transforms/IPO/FunctionImport.cpp
@@ -2006,6 +2006,17 @@ static bool doImportingForModuleForTest(
     ComputeCrossModuleImportForModuleForTest(M.getModuleIdentifier(),
                                              isPrevailing, *Index, ImportList);
 
+  // Conservatively mark all internal values as promoted. This interface is
+  // only used when doing importing via the function importing pass. The pass
+  // is only enabled when testing importing via the 'opt' tool, which does
+  // not do the ThinLink that would normally determine what values to promote.
+  for (auto &I : *Index) {
+    for (auto &S : I.second.SummaryList) {
+      if (GlobalValue::isLocalLinkage(S->linkage()))
+        S->setLinkage(GlobalValue::ExternalLinkage);
+    }
+  }
+
   // Next we need to promote to global scope and rename any local values that
   // are potentially exported to other modules.
   if (renameModuleForThinLTO(M, *Index, /*ClearDSOLocalOnDeclarations=*/false,
@@ -2014,20 +2025,6 @@ static bool doImportingForModuleForTest(
     return true;
   }
 
-  // Perform internalization of GVs in this module, if they are marked as
-  // internal by thinLTO summary.
-  GVSummaryMapTy DefinedGlobals;
-  for (const auto &GlobalList : *Index) {
-    auto GUID = GlobalList.first;
-    for (const auto &Summary : GlobalList.second.SummaryList) {
-      if (Summary->modulePath() == M.getName()) {
-        DefinedGlobals[GUID] = Summary.get();
-      }
-    }
-  }
-  thinLTOFinalizeInModule(M, DefinedGlobals, /*PropagateAttrs=*/true);
-  thinLTOInternalizeModule(M, DefinedGlobals);
-
   // Perform the import now.
   auto ModuleLoader = [&M](StringRef Identifier) {
     return loadFile(std::string(Identifier), M.getContext());
diff --git a/llvm/test/ThinLTO/X86/ditemplatevalueparameter-remap.ll b/llvm/test/ThinLTO/X86/ditemplatevalueparameter-remap.ll
index c462232b90cfd6..a008485dfbfd81 100644
--- a/llvm/test/ThinLTO/X86/ditemplatevalueparameter-remap.ll
+++ b/llvm/test/ThinLTO/X86/ditemplatevalueparameter-remap.ll
@@ -35,15 +35,13 @@
 ;
 ; RUN: opt -module-summary -o %t1.bc %s
 ; RUN: opt -module-summary -o %t2.bc %S/Inputs/ditemplatevalueparameter-remap.ll
-; RUN: llvm-lto2 run --thinlto-distributed-indexes %t1.bc %t2.bc -o %t3.o -save-temps \
+; RUN: llvm-lto2 run %t1.bc %t2.bc -o %t3 -save-temps \
 ; RUN:   -r=%t1.bc,_Z5func1v,p    \
 ; RUN:   -r=%t1.bc,_Z3bazv,px     \
 ; RUN:   -r=%t1.bc,_Z8thinlto1v,x \
 ; RUN:   -r=%t1.bc,_Z3barv,px     \
 ; RUN:   -r=%t2.bc,_Z8thinlto1v,px
-; RUN: opt -passes='function-import,module-inline' -enable-import-metadata \
-; RUN:   -import-all-index -summary-file=%t1.bc.thinlto.bc %t1.bc -o %t1.o
-; RUN: llvm-dis %t1.o -o - | FileCheck %s
+; RUN: llvm-dis %t3.1.4.opt.bc -o - | FileCheck %s
 
 target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-i128:128-f80:128-n8:16:32:64-S128"
 target triple = "x86_64-unknown-linux-gnu"
@@ -65,7 +63,7 @@ declare void @_Z8thinlto1v() unnamed_addr
 ; Check _Z8thinlto1v is inlined after thinLTO.
 ; CHECK: void @_Z3barv()
 ; CHECK-NOT: @_Z8thinlto1v()
-; CHECK: ret void
+; CHECK-NEXT: ret void
 define void @_Z3barv() unnamed_addr !dbg !14 {
   tail call void @_Z8thinlto1v(), !dbg !25
   ret void



More information about the llvm-commits mailing list