[lld] [llvm] [LLD] [COFF] Add a test to observe an unwanted effect of 3ab6209. NFC. (PR #72764)

Sun Nov 19 02:41:10 PST 2023

https://github.com/mstorsjo updated https://github.com/llvm/llvm-project/pull/72764

>From 138e915176877cb629dc787ad29b1b5b1a964f82 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Martin=20Storsj=C3=B6?= <martin at martin.st>
Date: Sun, 19 Nov 2023 12:18:49 +0200
Subject: [PATCH] [LLD] [COFF] Add tests to observe details about LTO and
 __imp_ prefixes. NFC.

The commit 3ab6209a3f93bdbeec8e9b9fcc00a9a4980915ff had the undesired
effect of retaining every __imp_ symbol, even if it isn't referenced
in any way. Add a testcase to observe this behaviour, to serve as
a reference point if this behaviour were to be improved later.

Port the testcase from b963c0b658cc54b370832df4f5a3d63fd69da334 from
the llvm/LTO testsuite into LLD as a preparation for changing that fix;
the moved testcase has a comment for one case which doesn't work
currently.

The testcase is ported mostly as is, but with symbol mangling simplified,
rewriting function names from MSVC C++ mangling to plain C, and
unnecessary debug info is removed.

Also extend the testcase to test combinations of both regular
object files and LTO objects. Leave out one combination which currently
fails, with a comment.
---
 lld/test/COFF/lto-dllimport.ll        | 44 +++++++++++++++++++++++++++
 lld/test/COFF/lto-imp-prefix.ll       | 17 ++++++++++-
 llvm/test/LTO/X86/Inputs/dllimport.ll | 17 -----------
 llvm/test/LTO/X86/dllimport.ll        | 30 ------------------
 4 files changed, 60 insertions(+), 48 deletions(-)
 create mode 100644 lld/test/COFF/lto-dllimport.ll
 delete mode 100644 llvm/test/LTO/X86/Inputs/dllimport.ll
 delete mode 100644 llvm/test/LTO/X86/dllimport.ll

diff --git a/lld/test/COFF/lto-dllimport.ll b/lld/test/COFF/lto-dllimport.ll
new file mode 100644
index 000000000000000..f3606899838b17a
--- /dev/null
+++ b/lld/test/COFF/lto-dllimport.ll
@@ -0,0 +1,44 @@
+; REQUIRES: x86
+; RUN: split-file %s %t.dir
+
+; RUN: llvm-as %t.dir/main.ll -o %t.main.bc
+; RUN: llvm-as %t.dir/other.ll -o %t.other.bc
+; RUN: llc %t.dir/main.ll -o %t.main.obj --filetype=obj
+; RUN: llc %t.dir/other.ll -o %t.other.obj --filetype=obj
+
+; RUN: lld-link %t.main.obj %t.other.obj -entry:main -out:%t.exe
+; RUN: lld-link %t.main.bc  %t.other.bc  -entry:main -out:%t.exe
+; RUN: lld-link %t.main.bc  %t.other.obj -entry:main -out:%t.exe
+
+;; This test currently fails if combining the regular object file main.obj
+;; with the bitcode object file other.bc.
+; RUN-skipped: lld-link %t.main.obj %t.other.bc  -entry:main -out:%t.exe
+
+;--- main.ll
+target datalayout = "e-m:w-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-pc-windows-msvc19.11.0"
+
+define dso_local i32 @main() local_unnamed_addr {
+entry:
+  %call = tail call i32 @foo()
+  ret i32 %call
+}
+
+declare dllimport i32 @foo() local_unnamed_addr
+
+!llvm.module.flags = !{!1}
+
+!1 = !{i32 1, !"ThinLTO", i32 0}
+
+;--- other.ll
+target datalayout = "e-m:w-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-pc-windows-msvc19.11.0"
+
+define dso_local i32 @foo() local_unnamed_addr {
+entry:
+  ret i32 42
+}
+
+!llvm.module.flags = !{!1}
+
+!1 = !{i32 1, !"ThinLTO", i32 0}
diff --git a/lld/test/COFF/lto-imp-prefix.ll b/lld/test/COFF/lto-imp-prefix.ll
index 503e4fd0649e41d..c0b3bea7841c435 100644
--- a/lld/test/COFF/lto-imp-prefix.ll
+++ b/lld/test/COFF/lto-imp-prefix.ll
@@ -5,7 +5,15 @@
 ; RUN: llvm-as %t.dir/main.ll -o %t.main.obj
 ; RUN: llvm-as %t.dir/other.ll -o %t.other.obj
 
-; RUN: lld-link /entry:entry %t.main.obj %t.other.obj /out:%t.exe /subsystem:console
+; RUN: lld-link /entry:entry %t.main.obj %t.other.obj /out:%t.exe /subsystem:console /debug:symtab
+
+;; The current implementation for handling __imp_ symbols retains all of them.
+;; Observe that this currently produces __imp_unusedFunc even if nothing
+;; references unusedFunc in any form.
+
+; RUN: llvm-nm %t.exe | FileCheck %s
+
+; CHECK: __imp_unusedFunc
 
 ;--- main.ll
 target datalayout = "e-m:w-p270:32:32-p271:32:32-p272:64:64-i64:64-i128:128-f80:128-n8:16:32:64-S128"
@@ -33,6 +41,13 @@ entry:
   ret void
 }
 
+ at __imp_unusedFunc = global ptr @unusedFuncReplacement
+
+define internal void @unusedFuncReplacement() {
+entry:
+  ret void
+}
+
 define void @other() {
 entry:
   ret void
diff --git a/llvm/test/LTO/X86/Inputs/dllimport.ll b/llvm/test/LTO/X86/Inputs/dllimport.ll
deleted file mode 100644
index f3daab1df09742d..000000000000000
--- a/llvm/test/LTO/X86/Inputs/dllimport.ll
+++ /dev/null
@@ -1,17 +0,0 @@
-; ModuleID = 'b.obj'
-source_filename = "b.cpp"
-target datalayout = "e-m:w-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
-target triple = "x86_64-pc-windows-msvc19.11.0"
-
-; Function Attrs: norecurse nounwind readnone sspstrong uwtable
-define dso_local i32 @"?foo@@YAHXZ"() local_unnamed_addr {
-entry:
-  ret i32 42
-}
-
-!llvm.module.flags = !{!1}
-
-!1 = !{i32 1, !"ThinLTO", i32 0}
-
-^0 = module: (path: "b.obj", hash: (0, 0, 0, 0, 0))
-^1 = gv: (name: "?foo@@YAHXZ", summaries: (function: (module: ^0, flags: (linkage: external, notEligibleToImport: 1, live: 0, dsoLocal: 1), insts: 1, funcFlags: (readNone: 1, readOnly: 0, noRecurse: 1, returnDoesNotAlias: 0)))) ; guid = 2709792123250749187
diff --git a/llvm/test/LTO/X86/dllimport.ll b/llvm/test/LTO/X86/dllimport.ll
deleted file mode 100644
index eb9f7df5c8b30a5..000000000000000
--- a/llvm/test/LTO/X86/dllimport.ll
+++ /dev/null
@@ -1,30 +0,0 @@
-; Test requiring LTO to remove the __imp_ prefix for locally imported COFF
-; symbols (mirroring how lld handles these symbols).
-; RUN: llvm-as %s -o %t.obj
-; RUN: llvm-as %S/Inputs/dllimport.ll -o %t2.obj
-; RUN: llvm-lto2 run -r=%t.obj,main,px -r %t.obj,__imp_?foo@@YAHXZ -r %t2.obj,?foo@@YAHXZ,p -o %t3 %t.obj %t2.obj -save-temps
-; RUN: llvm-dis %t3.0.0.preopt.bc -o - | FileCheck %s
-
-; CHECK: define dso_local i32 @"?foo@@YAHXZ"()
-
-; ModuleID = 'a.obj'
-source_filename = "a.cpp"
-target datalayout = "e-m:w-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
-target triple = "x86_64-pc-windows-msvc19.11.0"
-
-; Function Attrs: norecurse nounwind sspstrong uwtable
-define dso_local i32 @main() local_unnamed_addr {
-entry:
-  %call = tail call i32 @"?foo@@YAHXZ"()
-  ret i32 %call
-}
-
-declare dllimport i32 @"?foo@@YAHXZ"() local_unnamed_addr
-
-!llvm.module.flags = !{!1}
-
-!1 = !{i32 1, !"ThinLTO", i32 0}
-
-^0 = module: (path: "a.obj", hash: (0, 0, 0, 0, 0))
-^1 = gv: (name: "?foo@@YAHXZ") ; guid = 2709792123250749187
-^2 = gv: (name: "main", summaries: (function: (module: ^0, flags: (linkage: external, notEligibleToImport: 1, live: 0, dsoLocal: 1), insts: 2, funcFlags: (readNone: 0, readOnly: 0, noRecurse: 1, returnDoesNotAlias: 0), calls: ((callee: ^1))))) ; guid = 15822663052811949562

