Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Revert "[ThinLTO] Do not duplicate import a function that is actually defined in the current module" #111919

Merged
merged 1 commit into from
Oct 10, 2024

Conversation

huangjd
Copy link
Contributor

@huangjd huangjd commented Oct 10, 2024

Reverts #110064

@llvmbot llvmbot added the LTO Link time optimization (regular/full LTO or ThinLTO) label Oct 10, 2024
@llvmbot
Copy link
Member

llvmbot commented Oct 10, 2024

@llvm/pr-subscribers-lto

Author: William Junda Huang (huangjd)

Changes

Reverts llvm/llvm-project#110064


Full diff: https://github.com/llvm/llvm-project/pull/111919.diff

3 Files Affected:

  • (modified) llvm/lib/Linker/IRMover.cpp (+1-5)
  • (removed) llvm/test/ThinLTO/X86/Inputs/ditemplatevalueparameter-remap.ll (-29)
  • (removed) llvm/test/ThinLTO/X86/ditemplatevalueparameter-remap.ll (-87)
diff --git a/llvm/lib/Linker/IRMover.cpp b/llvm/lib/Linker/IRMover.cpp
index 5067fbff2e277b..3a6c2678cd157f 100644
--- a/llvm/lib/Linker/IRMover.cpp
+++ b/llvm/lib/Linker/IRMover.cpp
@@ -595,15 +595,11 @@ Value *IRLinker::materialize(Value *V, bool ForIndirectSymbol) {
   if (!SGV)
     return nullptr;
 
-  // If SGV is from dest, it was 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() != SrcM.get())
+  if (SGV->getParent() != &DstM && SGV->getParent() != SrcM.get())
     return nullptr;
 
   Expected<Constant *> NewProto = linkGlobalValueProto(SGV, ForIndirectSymbol);
diff --git a/llvm/test/ThinLTO/X86/Inputs/ditemplatevalueparameter-remap.ll b/llvm/test/ThinLTO/X86/Inputs/ditemplatevalueparameter-remap.ll
deleted file mode 100644
index be93160b943397..00000000000000
--- a/llvm/test/ThinLTO/X86/Inputs/ditemplatevalueparameter-remap.ll
+++ /dev/null
@@ -1,29 +0,0 @@
-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
deleted file mode 100644
index 0651705ccba8b8..00000000000000
--- a/llvm/test/ThinLTO/X86/ditemplatevalueparameter-remap.ll
+++ /dev/null
@@ -1,87 +0,0 @@
-; 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)

@huangjd
Copy link
Contributor Author

huangjd commented Oct 10, 2024

It seems a command used in test case is not available on some machines, reverting it to fix it

@huangjd huangjd merged commit 1bf271d into main Oct 10, 2024
7 of 9 checks passed
@huangjd huangjd deleted the revert-110064-thinLTONoImportSymbolFromCurrentModule branch October 10, 2024 23:10
DanielCChen pushed a commit to DanielCChen/llvm-project that referenced this pull request Oct 16, 2024
bricknerb pushed a commit to bricknerb/llvm-project that referenced this pull request Oct 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
LTO Link time optimization (regular/full LTO or ThinLTO)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants