[PATCH] D152207: [HIP] Instruct lld to go through all archives

2023-06-09 Thread Siu Chi Chan via Phabricator via cfe-commits 
This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGf1aee32f1c85: [HIP] Instruct lld to go through all archives 
(authored by scchan).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D152207/new/

https://reviews.llvm.org/D152207

Files:
  clang/lib/Driver/ToolChains/HIPAMD.cpp
  clang/test/Driver/hip-toolchain-rdc-separate.hip
  clang/test/Driver/hip-toolchain-rdc-static-lib.hip


Index: clang/test/Driver/hip-toolchain-rdc-static-lib.hip
===
--- clang/test/Driver/hip-toolchain-rdc-static-lib.hip
+++ clang/test/Driver/hip-toolchain-rdc-static-lib.hip
@@ -80,7 +80,9 @@
 // CHECK-NOT: ".*llc"
 // CHECK: [[LLD]] {{.*}} "-plugin-opt=-amdgpu-internalize-symbols"
 // CHECK-SAME: "-plugin-opt=mcpu=gfx900"
+// CHECK-SAME: "--whole-archive"
 // CHECK-SAME: "-o" "[[IMG_DEV2:.*out]]" [[A_BC2]] [[B_BC2]]
+// CHECK-SAME: "--no-whole-archive"
 
 // combine images generated into hip fat binary object
 // CHECK: [[BUNDLER:".*clang-offload-bundler"]] "-type=o"
Index: clang/test/Driver/hip-toolchain-rdc-separate.hip
===
--- clang/test/Driver/hip-toolchain-rdc-separate.hip
+++ clang/test/Driver/hip-toolchain-rdc-separate.hip
@@ -126,18 +126,22 @@
 // LINK-NOT: ".*llc"
 // LINK: {{".*lld.*"}} {{.*}} "-plugin-opt=-amdgpu-internalize-symbols"
 // LINK-SAME: "-plugin-opt=mcpu=gfx803"
+// LINK-SAME: "--whole-archive"
 // LLD-TMP-SAME: "-o" "[[IMG_DEV1:.*.out]]"
 // LLD-FIN-SAME: "-o" "[[IMG_DEV1:a.out-.*gfx803]]"
 // LINK-SAME "[[A_BC1]]" "[[B_BC1]]"
+// LINK-SAME: "--no-whole-archive"
 
 // LINK-NOT: "*.llvm-link"
 // LINK-NOT: ".*opt"
 // LINK-NOT: ".*llc"
 // LINK: {{".*lld.*"}} {{.*}} "-plugin-opt=-amdgpu-internalize-symbols"
 // LINK-SAME: "-plugin-opt=mcpu=gfx900"
+// LINK-SAME: "--whole-archive"
 // LLD-TMP-SAME: "-o" "[[IMG_DEV2:.*.out]]"
 // LLD-FIN-SAME: "-o" "[[IMG_DEV1:a.out-.*gfx900]]"
 // LINK-SAME "[[A_BC2]]" "[[B_BC2]]"
+// LINK-SAME: "--no-whole-archive"
 
 // LINK-BUNDLE: [[BUNDLER:".*clang-offload-bundler"]] "-type=o"
 // LINK-BUNDLE-SAME: 
"-targets={{.*}},hipv4-amdgcn-amd-amdhsa--gfx803,hipv4-amdgcn-amd-amdhsa--gfx900"
Index: clang/lib/Driver/ToolChains/HIPAMD.cpp
===
--- clang/lib/Driver/ToolChains/HIPAMD.cpp
+++ clang/lib/Driver/ToolChains/HIPAMD.cpp
@@ -152,6 +152,18 @@
 
   addLinkerCompressDebugSectionsOption(TC, Args, LldArgs);
 
+  // Given that host and device linking happen in separate processes, the 
device
+  // linker doesn't always have the visibility as to which device symbols are
+  // needed by a program, especially for the device symbol dependencies that 
are
+  // introduced through the host symbol resolution.
+  // For example: host_A() (A.obj) --> host_B(B.obj) --> device_kernel_B()
+  // (B.obj) In this case, the device linker doesn't know that A.obj actually
+  // depends on the kernel functions in B.obj.  When linking to static device
+  // library, the device linker may drop some of the device global symbols if
+  // they aren't referenced.  As a workaround, we are adding to the
+  // --whole-archive flag such that all global symbols would be linked in.
+  LldArgs.push_back("--whole-archive");
+
   for (auto *Arg : Args.filtered(options::OPT_Xoffload_linker)) {
 LldArgs.push_back(Arg->getValue(1));
 Arg->claim();
@@ -169,6 +181,8 @@
  /*IsBitCodeSDL=*/true,
  /*PostClangLink=*/false);
 
+  LldArgs.push_back("--no-whole-archive");
+
   const char *Lld = Args.MakeArgString(getToolChain().GetProgramPath("lld"));
   C.addCommand(std::make_unique(JA, *this, 
ResponseFileSupport::None(),
  Lld, LldArgs, Inputs, Output));


Index: clang/test/Driver/hip-toolchain-rdc-static-lib.hip
===
--- clang/test/Driver/hip-toolchain-rdc-static-lib.hip
+++ clang/test/Driver/hip-toolchain-rdc-static-lib.hip
@@ -80,7 +80,9 @@
 // CHECK-NOT: ".*llc"
 // CHECK: [[LLD]] {{.*}} "-plugin-opt=-amdgpu-internalize-symbols"
 // CHECK-SAME: "-plugin-opt=mcpu=gfx900"
+// CHECK-SAME: "--whole-archive"
 // CHECK-SAME: "-o" "[[IMG_DEV2:.*out]]" [[A_BC2]] [[B_BC2]]
+// CHECK-SAME: "--no-whole-archive"
 
 // combine images generated into hip fat binary object
 // CHECK: [[BUNDLER:".*clang-offload-bundler"]] "-type=o"
Index: clang/test/Driver/hip-toolchain-rdc-separate.hip
===
--- clang/test/Driver/hip-toolchain-rdc-separate.hip
+++ clang/test/Driver/hip-toolchain-rdc-separate.hip
@@ -126,18 +126,22 @@
 // LINK-NOT: ".*llc"
 // LINK: {{".*lld.*"}} {{.*}} "-plugin-opt=-amdgpu-internalize-symbols"
 // LINK-SAME: "-plugin-opt=mcpu=gfx803"
+// LINK-SAME: "--whole-archive"

[PATCH] D152207: [HIP] Instruct lld to go through all archives

2023-06-08 Thread Siu Chi Chan via Phabricator via cfe-commits 
scchan updated this revision to Diff 529667.
scchan added a comment.

rebased


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D152207/new/

https://reviews.llvm.org/D152207

Files:
  clang/lib/Driver/ToolChains/HIPAMD.cpp
  clang/test/Driver/hip-toolchain-rdc-separate.hip
  clang/test/Driver/hip-toolchain-rdc-static-lib.hip


Index: clang/test/Driver/hip-toolchain-rdc-static-lib.hip
===
--- clang/test/Driver/hip-toolchain-rdc-static-lib.hip
+++ clang/test/Driver/hip-toolchain-rdc-static-lib.hip
@@ -80,7 +80,9 @@
 // CHECK-NOT: ".*llc"
 // CHECK: [[LLD]] {{.*}} "-plugin-opt=-amdgpu-internalize-symbols"
 // CHECK-SAME: "-plugin-opt=mcpu=gfx900"
+// CHECK-SAME: "--whole-archive"
 // CHECK-SAME: "-o" "[[IMG_DEV2:.*out]]" [[A_BC2]] [[B_BC2]]
+// CHECK-SAME: "--no-whole-archive"
 
 // combine images generated into hip fat binary object
 // CHECK: [[BUNDLER:".*clang-offload-bundler"]] "-type=o"
Index: clang/test/Driver/hip-toolchain-rdc-separate.hip
===
--- clang/test/Driver/hip-toolchain-rdc-separate.hip
+++ clang/test/Driver/hip-toolchain-rdc-separate.hip
@@ -126,18 +126,22 @@
 // LINK-NOT: ".*llc"
 // LINK: {{".*lld.*"}} {{.*}} "-plugin-opt=-amdgpu-internalize-symbols"
 // LINK-SAME: "-plugin-opt=mcpu=gfx803"
+// LINK-SAME: "--whole-archive"
 // LLD-TMP-SAME: "-o" "[[IMG_DEV1:.*.out]]"
 // LLD-FIN-SAME: "-o" "[[IMG_DEV1:a.out-.*gfx803]]"
 // LINK-SAME "[[A_BC1]]" "[[B_BC1]]"
+// LINK-SAME: "--no-whole-archive"
 
 // LINK-NOT: "*.llvm-link"
 // LINK-NOT: ".*opt"
 // LINK-NOT: ".*llc"
 // LINK: {{".*lld.*"}} {{.*}} "-plugin-opt=-amdgpu-internalize-symbols"
 // LINK-SAME: "-plugin-opt=mcpu=gfx900"
+// LINK-SAME: "--whole-archive"
 // LLD-TMP-SAME: "-o" "[[IMG_DEV2:.*.out]]"
 // LLD-FIN-SAME: "-o" "[[IMG_DEV1:a.out-.*gfx900]]"
 // LINK-SAME "[[A_BC2]]" "[[B_BC2]]"
+// LINK-SAME: "--no-whole-archive"
 
 // LINK-BUNDLE: [[BUNDLER:".*clang-offload-bundler"]] "-type=o"
 // LINK-BUNDLE-SAME: 
"-targets={{.*}},hipv4-amdgcn-amd-amdhsa--gfx803,hipv4-amdgcn-amd-amdhsa--gfx900"
Index: clang/lib/Driver/ToolChains/HIPAMD.cpp
===
--- clang/lib/Driver/ToolChains/HIPAMD.cpp
+++ clang/lib/Driver/ToolChains/HIPAMD.cpp
@@ -152,6 +152,18 @@
 
   addLinkerCompressDebugSectionsOption(TC, Args, LldArgs);
 
+  // Given that host and device linking happen in separate processes, the 
device
+  // linker doesn't always have the visibility as to which device symbols are
+  // needed by a program, especially for the device symbol dependencies that 
are
+  // introduced through the host symbol resolution.
+  // For example: host_A() (A.obj) --> host_B(B.obj) --> device_kernel_B()
+  // (B.obj) In this case, the device linker doesn't know that A.obj actually
+  // depends on the kernel functions in B.obj.  When linking to static device
+  // library, the device linker may drop some of the device global symbols if
+  // they aren't referenced.  As a workaround, we are adding to the
+  // --whole-archive flag such that all global symbols would be linked in.
+  LldArgs.push_back("--whole-archive");
+
   for (auto *Arg : Args.filtered(options::OPT_Xoffload_linker)) {
 LldArgs.push_back(Arg->getValue(1));
 Arg->claim();
@@ -169,6 +181,8 @@
  /*IsBitCodeSDL=*/true,
  /*PostClangLink=*/false);
 
+  LldArgs.push_back("--no-whole-archive");
+
   const char *Lld = Args.MakeArgString(getToolChain().GetProgramPath("lld"));
   C.addCommand(std::make_unique(JA, *this, 
ResponseFileSupport::None(),
  Lld, LldArgs, Inputs, Output));


Index: clang/test/Driver/hip-toolchain-rdc-static-lib.hip
===
--- clang/test/Driver/hip-toolchain-rdc-static-lib.hip
+++ clang/test/Driver/hip-toolchain-rdc-static-lib.hip
@@ -80,7 +80,9 @@
 // CHECK-NOT: ".*llc"
 // CHECK: [[LLD]] {{.*}} "-plugin-opt=-amdgpu-internalize-symbols"
 // CHECK-SAME: "-plugin-opt=mcpu=gfx900"
+// CHECK-SAME: "--whole-archive"
 // CHECK-SAME: "-o" "[[IMG_DEV2:.*out]]" [[A_BC2]] [[B_BC2]]
+// CHECK-SAME: "--no-whole-archive"
 
 // combine images generated into hip fat binary object
 // CHECK: [[BUNDLER:".*clang-offload-bundler"]] "-type=o"
Index: clang/test/Driver/hip-toolchain-rdc-separate.hip
===
--- clang/test/Driver/hip-toolchain-rdc-separate.hip
+++ clang/test/Driver/hip-toolchain-rdc-separate.hip
@@ -126,18 +126,22 @@
 // LINK-NOT: ".*llc"
 // LINK: {{".*lld.*"}} {{.*}} "-plugin-opt=-amdgpu-internalize-symbols"
 // LINK-SAME: "-plugin-opt=mcpu=gfx803"
+// LINK-SAME: "--whole-archive"
 // LLD-TMP-SAME: "-o" "[[IMG_DEV1:.*.out]]"
 // LLD-FIN-SAME: "-o" "[[IMG_DEV1:a.out-.*gfx803]]"
 // LINK-SAME "[[A_BC1]]" "[[B_BC1]]"
+// LINK-SAME:

[PATCH] D152207: [HIP] Instruct lld to go through all archives

2023-06-07 Thread Siu Chi Chan via Phabricator via cfe-commits 
scchan updated this revision to Diff 529400.
scchan added a comment.

Removed redundant comment.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D152207/new/

https://reviews.llvm.org/D152207

Files:
  clang/lib/Driver/ToolChains/HIPAMD.cpp
  clang/test/Driver/hip-toolchain-rdc-separate.hip
  clang/test/Driver/hip-toolchain-rdc-static-lib.hip


Index: clang/test/Driver/hip-toolchain-rdc-static-lib.hip
===
--- clang/test/Driver/hip-toolchain-rdc-static-lib.hip
+++ clang/test/Driver/hip-toolchain-rdc-static-lib.hip
@@ -80,7 +80,9 @@
 // CHECK-NOT: ".*llc"
 // CHECK: [[LLD]] {{.*}} "-plugin-opt=-amdgpu-internalize-symbols"
 // CHECK-SAME: "-plugin-opt=mcpu=gfx900"
+// CHECK-SAME: "--whole-archive"
 // CHECK-SAME: "-o" "[[IMG_DEV2:.*out]]" [[A_BC2]] [[B_BC2]]
+// CHECK-SAME: "--no-whole-archive"
 
 // combine images generated into hip fat binary object
 // CHECK: [[BUNDLER:".*clang-offload-bundler"]] "-type=o"
Index: clang/test/Driver/hip-toolchain-rdc-separate.hip
===
--- clang/test/Driver/hip-toolchain-rdc-separate.hip
+++ clang/test/Driver/hip-toolchain-rdc-separate.hip
@@ -126,18 +126,22 @@
 // LINK-NOT: ".*llc"
 // LINK: {{".*lld.*"}} {{.*}} "-plugin-opt=-amdgpu-internalize-symbols"
 // LINK-SAME: "-plugin-opt=mcpu=gfx803"
+// LINK-SAME: "--whole-archive"
 // LLD-TMP-SAME: "-o" "[[IMG_DEV1:.*.out]]"
 // LLD-FIN-SAME: "-o" "[[IMG_DEV1:a.out-.*gfx803]]"
 // LINK-SAME "[[A_BC1]]" "[[B_BC1]]"
+// LINK-SAME: "--no-whole-archive"
 
 // LINK-NOT: "*.llvm-link"
 // LINK-NOT: ".*opt"
 // LINK-NOT: ".*llc"
 // LINK: {{".*lld.*"}} {{.*}} "-plugin-opt=-amdgpu-internalize-symbols"
 // LINK-SAME: "-plugin-opt=mcpu=gfx900"
+// LINK-SAME: "--whole-archive"
 // LLD-TMP-SAME: "-o" "[[IMG_DEV2:.*.out]]"
 // LLD-FIN-SAME: "-o" "[[IMG_DEV1:a.out-.*gfx900]]"
 // LINK-SAME "[[A_BC2]]" "[[B_BC2]]"
+// LINK-SAME: "--no-whole-archive"
 
 // LINK-BUNDLE: [[BUNDLER:".*clang-offload-bundler"]] "-type=o"
 // LINK-BUNDLE-SAME: 
"-targets={{.*}},hipv4-amdgcn-amd-amdhsa--gfx803,hipv4-amdgcn-amd-amdhsa--gfx900"
Index: clang/lib/Driver/ToolChains/HIPAMD.cpp
===
--- clang/lib/Driver/ToolChains/HIPAMD.cpp
+++ clang/lib/Driver/ToolChains/HIPAMD.cpp
@@ -152,6 +152,18 @@
 
   addLinkerCompressDebugSectionsOption(TC, Args, LldArgs);
 
+  // Given that host and device linking happen in separate processes, the 
device
+  // linker doesn't always have the visibility as to which device symbols are
+  // needed by a program, especially for the device symbol dependencies that 
are
+  // introduced through the host symbol resolution.
+  // For example: host_A() (A.obj) --> host_B(B.obj) --> device_kernel_B()
+  // (B.obj) In this case, the device linker doesn't know that A.obj actually
+  // depends on the kernel functions in B.obj.  When linking to static device
+  // library, the device linker may drop some of the device global symbols if
+  // they aren't referenced.  As a workaround, we are adding to the
+  // --whole-archive flag such that all global symbols would be linked in.
+  LldArgs.push_back("--whole-archive");
+
   for (auto *Arg : Args.filtered(options::OPT_Xoffload_linker)) {
 LldArgs.push_back(Arg->getValue(1));
 Arg->claim();
@@ -169,6 +181,8 @@
  /*IsBitCodeSDL=*/true,
  /*PostClangLink=*/false);
 
+  LldArgs.push_back("--no-whole-archive");
+
   const char *Lld = Args.MakeArgString(getToolChain().GetProgramPath("lld"));
   C.addCommand(std::make_unique(JA, *this, 
ResponseFileSupport::None(),
  Lld, LldArgs, Inputs, Output));


Index: clang/test/Driver/hip-toolchain-rdc-static-lib.hip
===
--- clang/test/Driver/hip-toolchain-rdc-static-lib.hip
+++ clang/test/Driver/hip-toolchain-rdc-static-lib.hip
@@ -80,7 +80,9 @@
 // CHECK-NOT: ".*llc"
 // CHECK: [[LLD]] {{.*}} "-plugin-opt=-amdgpu-internalize-symbols"
 // CHECK-SAME: "-plugin-opt=mcpu=gfx900"
+// CHECK-SAME: "--whole-archive"
 // CHECK-SAME: "-o" "[[IMG_DEV2:.*out]]" [[A_BC2]] [[B_BC2]]
+// CHECK-SAME: "--no-whole-archive"
 
 // combine images generated into hip fat binary object
 // CHECK: [[BUNDLER:".*clang-offload-bundler"]] "-type=o"
Index: clang/test/Driver/hip-toolchain-rdc-separate.hip
===
--- clang/test/Driver/hip-toolchain-rdc-separate.hip
+++ clang/test/Driver/hip-toolchain-rdc-separate.hip
@@ -126,18 +126,22 @@
 // LINK-NOT: ".*llc"
 // LINK: {{".*lld.*"}} {{.*}} "-plugin-opt=-amdgpu-internalize-symbols"
 // LINK-SAME: "-plugin-opt=mcpu=gfx803"
+// LINK-SAME: "--whole-archive"
 // LLD-TMP-SAME: "-o" "[[IMG_DEV1:.*.out]]"
 // LLD-FIN-SAME: "-o" "[[IMG_DEV1:a.out-.*gfx803]]"
 // LINK-SAME "[[A_BC1]]"

[PATCH] D152207: [HIP] Instruct lld to go through all archives

2023-06-07 Thread Fangrui Song via Phabricator via cfe-commits 
MaskRay added inline comments.



Comment at: clang/lib/Driver/ToolChains/HIPAMD.cpp:184
 
+  // pair with the --whole-archive being added previously
+  LldArgs.push_back("--no-whole-archive");

For complete sentences in comments, capitalize and add a full stop.

Actually, this statement is self-explanatory and I don't think a comment is 
useful.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D152207/new/

https://reviews.llvm.org/D152207

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D152207: [HIP] Instruct lld to go through all archives

2023-06-07 Thread Siu Chi Chan via Phabricator via cfe-commits 
scchan updated this revision to Diff 529366.
scchan added a comment.

added a matching --no-whole-archive


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D152207/new/

https://reviews.llvm.org/D152207

Files:
  clang/lib/Driver/ToolChains/HIPAMD.cpp
  clang/test/Driver/hip-toolchain-rdc-separate.hip
  clang/test/Driver/hip-toolchain-rdc-static-lib.hip


Index: clang/test/Driver/hip-toolchain-rdc-static-lib.hip
===
--- clang/test/Driver/hip-toolchain-rdc-static-lib.hip
+++ clang/test/Driver/hip-toolchain-rdc-static-lib.hip
@@ -80,7 +80,9 @@
 // CHECK-NOT: ".*llc"
 // CHECK: [[LLD]] {{.*}} "-plugin-opt=-amdgpu-internalize-symbols"
 // CHECK-SAME: "-plugin-opt=mcpu=gfx900"
+// CHECK-SAME: "--whole-archive"
 // CHECK-SAME: "-o" "[[IMG_DEV2:.*out]]" [[A_BC2]] [[B_BC2]]
+// CHECK-SAME: "--no-whole-archive"
 
 // combine images generated into hip fat binary object
 // CHECK: [[BUNDLER:".*clang-offload-bundler"]] "-type=o"
Index: clang/test/Driver/hip-toolchain-rdc-separate.hip
===
--- clang/test/Driver/hip-toolchain-rdc-separate.hip
+++ clang/test/Driver/hip-toolchain-rdc-separate.hip
@@ -126,18 +126,22 @@
 // LINK-NOT: ".*llc"
 // LINK: {{".*lld.*"}} {{.*}} "-plugin-opt=-amdgpu-internalize-symbols"
 // LINK-SAME: "-plugin-opt=mcpu=gfx803"
+// LINK-SAME: "--whole-archive"
 // LLD-TMP-SAME: "-o" "[[IMG_DEV1:.*.out]]"
 // LLD-FIN-SAME: "-o" "[[IMG_DEV1:a.out-.*gfx803]]"
 // LINK-SAME "[[A_BC1]]" "[[B_BC1]]"
+// LINK-SAME: "--no-whole-archive"
 
 // LINK-NOT: "*.llvm-link"
 // LINK-NOT: ".*opt"
 // LINK-NOT: ".*llc"
 // LINK: {{".*lld.*"}} {{.*}} "-plugin-opt=-amdgpu-internalize-symbols"
 // LINK-SAME: "-plugin-opt=mcpu=gfx900"
+// LINK-SAME: "--whole-archive"
 // LLD-TMP-SAME: "-o" "[[IMG_DEV2:.*.out]]"
 // LLD-FIN-SAME: "-o" "[[IMG_DEV1:a.out-.*gfx900]]"
 // LINK-SAME "[[A_BC2]]" "[[B_BC2]]"
+// LINK-SAME: "--no-whole-archive"
 
 // LINK-BUNDLE: [[BUNDLER:".*clang-offload-bundler"]] "-type=o"
 // LINK-BUNDLE-SAME: 
"-targets={{.*}},hipv4-amdgcn-amd-amdhsa--gfx803,hipv4-amdgcn-amd-amdhsa--gfx900"
Index: clang/lib/Driver/ToolChains/HIPAMD.cpp
===
--- clang/lib/Driver/ToolChains/HIPAMD.cpp
+++ clang/lib/Driver/ToolChains/HIPAMD.cpp
@@ -152,6 +152,18 @@
 
   addLinkerCompressDebugSectionsOption(TC, Args, LldArgs);
 
+  // Given that host and device linking happen in separate processes, the 
device
+  // linker doesn't always have the visibility as to which device symbols are
+  // needed by a program, especially for the device symbol dependencies that 
are
+  // introduced through the host symbol resolution.
+  // For example: host_A() (A.obj) --> host_B(B.obj) --> device_kernel_B()
+  // (B.obj) In this case, the device linker doesn't know that A.obj actually
+  // depends on the kernel functions in B.obj.  When linking to static device
+  // library, the device linker may drop some of the device global symbols if
+  // they aren't referenced.  As a workaround, we are adding to the
+  // --whole-archive flag such that all global symbols would be linked in.
+  LldArgs.push_back("--whole-archive");
+
   for (auto *Arg : Args.filtered(options::OPT_Xoffload_linker)) {
 LldArgs.push_back(Arg->getValue(1));
 Arg->claim();
@@ -169,6 +181,9 @@
  /*IsBitCodeSDL=*/true,
  /*PostClangLink=*/false);
 
+  // pair with the --whole-archive being added previously
+  LldArgs.push_back("--no-whole-archive");
+
   const char *Lld = Args.MakeArgString(getToolChain().GetProgramPath("lld"));
   C.addCommand(std::make_unique(JA, *this, 
ResponseFileSupport::None(),
  Lld, LldArgs, Inputs, Output));


Index: clang/test/Driver/hip-toolchain-rdc-static-lib.hip
===
--- clang/test/Driver/hip-toolchain-rdc-static-lib.hip
+++ clang/test/Driver/hip-toolchain-rdc-static-lib.hip
@@ -80,7 +80,9 @@
 // CHECK-NOT: ".*llc"
 // CHECK: [[LLD]] {{.*}} "-plugin-opt=-amdgpu-internalize-symbols"
 // CHECK-SAME: "-plugin-opt=mcpu=gfx900"
+// CHECK-SAME: "--whole-archive"
 // CHECK-SAME: "-o" "[[IMG_DEV2:.*out]]" [[A_BC2]] [[B_BC2]]
+// CHECK-SAME: "--no-whole-archive"
 
 // combine images generated into hip fat binary object
 // CHECK: [[BUNDLER:".*clang-offload-bundler"]] "-type=o"
Index: clang/test/Driver/hip-toolchain-rdc-separate.hip
===
--- clang/test/Driver/hip-toolchain-rdc-separate.hip
+++ clang/test/Driver/hip-toolchain-rdc-separate.hip
@@ -126,18 +126,22 @@
 // LINK-NOT: ".*llc"
 // LINK: {{".*lld.*"}} {{.*}} "-plugin-opt=-amdgpu-internalize-symbols"
 // LINK-SAME: "-plugin-opt=mcpu=gfx803"
+// LINK-SAME: "--whole-archive"
 // LLD-TMP-SAME: "-o" "[[IMG_DEV1:.*.out]]"
 // LLD-FIN-SAME:

[PATCH] D152207: [HIP] Instruct lld to go through all archives

2023-06-07 Thread Fangrui Song via Phabricator via cfe-commits 
MaskRay added inline comments.



Comment at: clang/lib/Driver/ToolChains/HIPAMD.cpp:165
+  // --whole-archive flag such that all global symbols would be linked in.
+  LldArgs.push_back("--whole-archive");
+

Though not strictly required, it's usually better to end `--whole-archive` with 
a pairing `--no-whole-archive`.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D152207/new/

https://reviews.llvm.org/D152207

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D152207: [HIP] Instruct lld to go through all archives

2023-06-07 Thread Siu Chi Chan via Phabricator via cfe-commits 
scchan updated this revision to Diff 529311.
scchan added a comment.

fix some formatting issue


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D152207/new/

https://reviews.llvm.org/D152207

Files:
  clang/lib/Driver/ToolChains/HIPAMD.cpp
  clang/test/Driver/hip-toolchain-rdc-separate.hip
  clang/test/Driver/hip-toolchain-rdc-static-lib.hip


Index: clang/test/Driver/hip-toolchain-rdc-static-lib.hip
===
--- clang/test/Driver/hip-toolchain-rdc-static-lib.hip
+++ clang/test/Driver/hip-toolchain-rdc-static-lib.hip
@@ -80,6 +80,7 @@
 // CHECK-NOT: ".*llc"
 // CHECK: [[LLD]] {{.*}} "-plugin-opt=-amdgpu-internalize-symbols"
 // CHECK-SAME: "-plugin-opt=mcpu=gfx900"
+// CHECK-SAME: "--whole-archive"
 // CHECK-SAME: "-o" "[[IMG_DEV2:.*out]]" [[A_BC2]] [[B_BC2]]
 
 // combine images generated into hip fat binary object
Index: clang/test/Driver/hip-toolchain-rdc-separate.hip
===
--- clang/test/Driver/hip-toolchain-rdc-separate.hip
+++ clang/test/Driver/hip-toolchain-rdc-separate.hip
@@ -126,6 +126,7 @@
 // LINK-NOT: ".*llc"
 // LINK: {{".*lld.*"}} {{.*}} "-plugin-opt=-amdgpu-internalize-symbols"
 // LINK-SAME: "-plugin-opt=mcpu=gfx803"
+// LINK-SAME: "--whole-archive"
 // LLD-TMP-SAME: "-o" "[[IMG_DEV1:.*.out]]"
 // LLD-FIN-SAME: "-o" "[[IMG_DEV1:a.out-.*gfx803]]"
 // LINK-SAME "[[A_BC1]]" "[[B_BC1]]"
@@ -135,6 +136,7 @@
 // LINK-NOT: ".*llc"
 // LINK: {{".*lld.*"}} {{.*}} "-plugin-opt=-amdgpu-internalize-symbols"
 // LINK-SAME: "-plugin-opt=mcpu=gfx900"
+// LINK-SAME: "--whole-archive"
 // LLD-TMP-SAME: "-o" "[[IMG_DEV2:.*.out]]"
 // LLD-FIN-SAME: "-o" "[[IMG_DEV1:a.out-.*gfx900]]"
 // LINK-SAME "[[A_BC2]]" "[[B_BC2]]"
Index: clang/lib/Driver/ToolChains/HIPAMD.cpp
===
--- clang/lib/Driver/ToolChains/HIPAMD.cpp
+++ clang/lib/Driver/ToolChains/HIPAMD.cpp
@@ -152,6 +152,18 @@
 
   addLinkerCompressDebugSectionsOption(TC, Args, LldArgs);
 
+  // Given that host and device linking happen in separate processes, the 
device
+  // linker doesn't always have the visibility as to which device symbols are
+  // needed by a program, especially for the device symbol dependencies that 
are
+  // introduced through the host symbol resolution.
+  // For example: host_A() (A.obj) --> host_B(B.obj) --> device_kernel_B()
+  // (B.obj) In this case, the device linker doesn't know that A.obj actually
+  // depends on the kernel functions in B.obj.  When linking to static device
+  // library, the device linker may drop some of the device global symbols if
+  // they aren't referenced.  As a workaround, we are adding to the
+  // --whole-archive flag such that all global symbols would be linked in.
+  LldArgs.push_back("--whole-archive");
+
   for (auto *Arg : Args.filtered(options::OPT_Xoffload_linker)) {
 LldArgs.push_back(Arg->getValue(1));
 Arg->claim();


Index: clang/test/Driver/hip-toolchain-rdc-static-lib.hip
===
--- clang/test/Driver/hip-toolchain-rdc-static-lib.hip
+++ clang/test/Driver/hip-toolchain-rdc-static-lib.hip
@@ -80,6 +80,7 @@
 // CHECK-NOT: ".*llc"
 // CHECK: [[LLD]] {{.*}} "-plugin-opt=-amdgpu-internalize-symbols"
 // CHECK-SAME: "-plugin-opt=mcpu=gfx900"
+// CHECK-SAME: "--whole-archive"
 // CHECK-SAME: "-o" "[[IMG_DEV2:.*out]]" [[A_BC2]] [[B_BC2]]
 
 // combine images generated into hip fat binary object
Index: clang/test/Driver/hip-toolchain-rdc-separate.hip
===
--- clang/test/Driver/hip-toolchain-rdc-separate.hip
+++ clang/test/Driver/hip-toolchain-rdc-separate.hip
@@ -126,6 +126,7 @@
 // LINK-NOT: ".*llc"
 // LINK: {{".*lld.*"}} {{.*}} "-plugin-opt=-amdgpu-internalize-symbols"
 // LINK-SAME: "-plugin-opt=mcpu=gfx803"
+// LINK-SAME: "--whole-archive"
 // LLD-TMP-SAME: "-o" "[[IMG_DEV1:.*.out]]"
 // LLD-FIN-SAME: "-o" "[[IMG_DEV1:a.out-.*gfx803]]"
 // LINK-SAME "[[A_BC1]]" "[[B_BC1]]"
@@ -135,6 +136,7 @@
 // LINK-NOT: ".*llc"
 // LINK: {{".*lld.*"}} {{.*}} "-plugin-opt=-amdgpu-internalize-symbols"
 // LINK-SAME: "-plugin-opt=mcpu=gfx900"
+// LINK-SAME: "--whole-archive"
 // LLD-TMP-SAME: "-o" "[[IMG_DEV2:.*.out]]"
 // LLD-FIN-SAME: "-o" "[[IMG_DEV1:a.out-.*gfx900]]"
 // LINK-SAME "[[A_BC2]]" "[[B_BC2]]"
Index: clang/lib/Driver/ToolChains/HIPAMD.cpp
===
--- clang/lib/Driver/ToolChains/HIPAMD.cpp
+++ clang/lib/Driver/ToolChains/HIPAMD.cpp
@@ -152,6 +152,18 @@
 
   addLinkerCompressDebugSectionsOption(TC, Args, LldArgs);
 
+  // Given that host and device linking happen in separate processes, the device
+  // linker doesn't always have the visibility as to which device symbols are
+  // needed by a program, especially for the device symbol dependencies that are
+  // introduced

[PATCH] D152207: [HIP] Instruct lld to go through all archives

2023-06-07 Thread Yaxun Liu via Phabricator via cfe-commits 
yaxunl accepted this revision.
yaxunl added a comment.
This revision is now accepted and ready to land.

LGTM. Thanks.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D152207/new/

https://reviews.llvm.org/D152207

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D152207: [HIP] Instruct lld to go through all archives

2023-06-06 Thread Siu Chi Chan via Phabricator via cfe-commits 
scchan updated this revision to Diff 529061.
scchan added a comment.

remove ws


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D152207/new/

https://reviews.llvm.org/D152207

Files:
  clang/lib/Driver/ToolChains/HIPAMD.cpp
  clang/test/Driver/hip-toolchain-rdc-separate.hip
  clang/test/Driver/hip-toolchain-rdc-static-lib.hip


Index: clang/test/Driver/hip-toolchain-rdc-static-lib.hip
===
--- clang/test/Driver/hip-toolchain-rdc-static-lib.hip
+++ clang/test/Driver/hip-toolchain-rdc-static-lib.hip
@@ -80,6 +80,7 @@
 // CHECK-NOT: ".*llc"
 // CHECK: [[LLD]] {{.*}} "-plugin-opt=-amdgpu-internalize-symbols"
 // CHECK-SAME: "-plugin-opt=mcpu=gfx900"
+// CHECK-SAME: "--whole-archive"
 // CHECK-SAME: "-o" "[[IMG_DEV2:.*out]]" [[A_BC2]] [[B_BC2]]
 
 // combine images generated into hip fat binary object
Index: clang/test/Driver/hip-toolchain-rdc-separate.hip
===
--- clang/test/Driver/hip-toolchain-rdc-separate.hip
+++ clang/test/Driver/hip-toolchain-rdc-separate.hip
@@ -126,6 +126,7 @@
 // LINK-NOT: ".*llc"
 // LINK: {{".*lld.*"}} {{.*}} "-plugin-opt=-amdgpu-internalize-symbols"
 // LINK-SAME: "-plugin-opt=mcpu=gfx803"
+// LINK-SAME: "--whole-archive"
 // LLD-TMP-SAME: "-o" "[[IMG_DEV1:.*.out]]"
 // LLD-FIN-SAME: "-o" "[[IMG_DEV1:a.out-.*gfx803]]"
 // LINK-SAME "[[A_BC1]]" "[[B_BC1]]"
@@ -135,6 +136,7 @@
 // LINK-NOT: ".*llc"
 // LINK: {{".*lld.*"}} {{.*}} "-plugin-opt=-amdgpu-internalize-symbols"
 // LINK-SAME: "-plugin-opt=mcpu=gfx900"
+// LINK-SAME: "--whole-archive"
 // LLD-TMP-SAME: "-o" "[[IMG_DEV2:.*.out]]"
 // LLD-FIN-SAME: "-o" "[[IMG_DEV1:a.out-.*gfx900]]"
 // LINK-SAME "[[A_BC2]]" "[[B_BC2]]"
Index: clang/lib/Driver/ToolChains/HIPAMD.cpp
===
--- clang/lib/Driver/ToolChains/HIPAMD.cpp
+++ clang/lib/Driver/ToolChains/HIPAMD.cpp
@@ -152,6 +152,18 @@
 
   addLinkerCompressDebugSectionsOption(TC, Args, LldArgs);
 
+  // Given that host and device linking happen in separate processes, the 
device
+  // linker doesn't always have the visibility as to which device symbols are
+  // needed by a program, especially for the device symbol dependencies that 
are
+  // introduced through the host symbol resolution.
+  // For example: host_A() (A.obj) --> host_B(B.obj) --> device_kernel_B() 
(B.obj)
+  // In this case, the device linker doesn't know that A.obj actually depends 
on 
+  // the kernel functions in B.obj.  When linking to static device library, 
the 
+  // device linker may drop some of the device global symbols if they aren't
+  // referenced.  As a workaround, we are adding to the --whole-archive flag 
such
+  // that all global symbols would be linked in.
+  LldArgs.push_back("--whole-archive");
+
   for (auto *Arg : Args.filtered(options::OPT_Xoffload_linker)) {
 LldArgs.push_back(Arg->getValue(1));
 Arg->claim();


Index: clang/test/Driver/hip-toolchain-rdc-static-lib.hip
===
--- clang/test/Driver/hip-toolchain-rdc-static-lib.hip
+++ clang/test/Driver/hip-toolchain-rdc-static-lib.hip
@@ -80,6 +80,7 @@
 // CHECK-NOT: ".*llc"
 // CHECK: [[LLD]] {{.*}} "-plugin-opt=-amdgpu-internalize-symbols"
 // CHECK-SAME: "-plugin-opt=mcpu=gfx900"
+// CHECK-SAME: "--whole-archive"
 // CHECK-SAME: "-o" "[[IMG_DEV2:.*out]]" [[A_BC2]] [[B_BC2]]
 
 // combine images generated into hip fat binary object
Index: clang/test/Driver/hip-toolchain-rdc-separate.hip
===
--- clang/test/Driver/hip-toolchain-rdc-separate.hip
+++ clang/test/Driver/hip-toolchain-rdc-separate.hip
@@ -126,6 +126,7 @@
 // LINK-NOT: ".*llc"
 // LINK: {{".*lld.*"}} {{.*}} "-plugin-opt=-amdgpu-internalize-symbols"
 // LINK-SAME: "-plugin-opt=mcpu=gfx803"
+// LINK-SAME: "--whole-archive"
 // LLD-TMP-SAME: "-o" "[[IMG_DEV1:.*.out]]"
 // LLD-FIN-SAME: "-o" "[[IMG_DEV1:a.out-.*gfx803]]"
 // LINK-SAME "[[A_BC1]]" "[[B_BC1]]"
@@ -135,6 +136,7 @@
 // LINK-NOT: ".*llc"
 // LINK: {{".*lld.*"}} {{.*}} "-plugin-opt=-amdgpu-internalize-symbols"
 // LINK-SAME: "-plugin-opt=mcpu=gfx900"
+// LINK-SAME: "--whole-archive"
 // LLD-TMP-SAME: "-o" "[[IMG_DEV2:.*.out]]"
 // LLD-FIN-SAME: "-o" "[[IMG_DEV1:a.out-.*gfx900]]"
 // LINK-SAME "[[A_BC2]]" "[[B_BC2]]"
Index: clang/lib/Driver/ToolChains/HIPAMD.cpp
===
--- clang/lib/Driver/ToolChains/HIPAMD.cpp
+++ clang/lib/Driver/ToolChains/HIPAMD.cpp
@@ -152,6 +152,18 @@
 
   addLinkerCompressDebugSectionsOption(TC, Args, LldArgs);
 
+  // Given that host and device linking happen in separate processes, the device
+  // linker doesn't always have the visibility as to which device symbols are
+  // needed by a program, especially for the device symbol dependencies that are
+  // introduced through the

[PATCH] D152207: [HIP] Instruct lld to go through all archives

2023-06-06 Thread Siu Chi Chan via Phabricator via cfe-commits 
scchan updated this revision to Diff 529060.
scchan added a comment.

Updated patch to address review feedback


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D152207/new/

https://reviews.llvm.org/D152207

Files:
  clang/lib/Driver/ToolChains/HIPAMD.cpp
  clang/test/Driver/hip-toolchain-rdc-separate.hip
  clang/test/Driver/hip-toolchain-rdc-static-lib.hip


Index: clang/test/Driver/hip-toolchain-rdc-static-lib.hip
===
--- clang/test/Driver/hip-toolchain-rdc-static-lib.hip
+++ clang/test/Driver/hip-toolchain-rdc-static-lib.hip
@@ -80,6 +80,7 @@
 // CHECK-NOT: ".*llc"
 // CHECK: [[LLD]] {{.*}} "-plugin-opt=-amdgpu-internalize-symbols"
 // CHECK-SAME: "-plugin-opt=mcpu=gfx900"
+// CHECK-SAME: "--whole-archive"
 // CHECK-SAME: "-o" "[[IMG_DEV2:.*out]]" [[A_BC2]] [[B_BC2]]
 
 // combine images generated into hip fat binary object
Index: clang/test/Driver/hip-toolchain-rdc-separate.hip
===
--- clang/test/Driver/hip-toolchain-rdc-separate.hip
+++ clang/test/Driver/hip-toolchain-rdc-separate.hip
@@ -126,6 +126,7 @@
 // LINK-NOT: ".*llc"
 // LINK: {{".*lld.*"}} {{.*}} "-plugin-opt=-amdgpu-internalize-symbols"
 // LINK-SAME: "-plugin-opt=mcpu=gfx803"
+// LINK-SAME: "--whole-archive"
 // LLD-TMP-SAME: "-o" "[[IMG_DEV1:.*.out]]"
 // LLD-FIN-SAME: "-o" "[[IMG_DEV1:a.out-.*gfx803]]"
 // LINK-SAME "[[A_BC1]]" "[[B_BC1]]"
@@ -135,10 +136,12 @@
 // LINK-NOT: ".*llc"
 // LINK: {{".*lld.*"}} {{.*}} "-plugin-opt=-amdgpu-internalize-symbols"
 // LINK-SAME: "-plugin-opt=mcpu=gfx900"
+// LINK-SAME: "--whole-archive"
 // LLD-TMP-SAME: "-o" "[[IMG_DEV2:.*.out]]"
 // LLD-FIN-SAME: "-o" "[[IMG_DEV1:a.out-.*gfx900]]"
 // LINK-SAME "[[A_BC2]]" "[[B_BC2]]"
 
+
 // LINK-BUNDLE: [[BUNDLER:".*clang-offload-bundler"]] "-type=o"
 // LINK-BUNDLE-SAME: 
"-targets={{.*}},hipv4-amdgcn-amd-amdhsa--gfx803,hipv4-amdgcn-amd-amdhsa--gfx900"
 // LINK-BUNDLE-SAME: "-input={{.*}}" "-input=[[IMG_DEV1]]" 
"-input=[[IMG_DEV2]]" "-output=[[BUNDLE:.*]]"
Index: clang/lib/Driver/ToolChains/HIPAMD.cpp
===
--- clang/lib/Driver/ToolChains/HIPAMD.cpp
+++ clang/lib/Driver/ToolChains/HIPAMD.cpp
@@ -152,6 +152,18 @@
 
   addLinkerCompressDebugSectionsOption(TC, Args, LldArgs);
 
+  // Given that host and device linking happen in separate processes, the 
device
+  // linker doesn't always have the visibility as to which device symbols are
+  // needed by a program, especially for the device symbol dependencies that 
are
+  // introduced through the host symbol resolution.
+  // For example: host_A() (A.obj) --> host_B(B.obj) --> device_kernel_B() 
(B.obj)
+  // In this case, the device linker doesn't know that A.obj actually depends 
on 
+  // the kernel functions in B.obj.  When linking to static device library, 
the 
+  // device linker may drop some of the device global symbols if they aren't
+  // referenced.  As a workaround, we are adding to the --whole-archive flag 
such
+  // that all global symbols would be linked in.
+  LldArgs.push_back("--whole-archive");
+
   for (auto *Arg : Args.filtered(options::OPT_Xoffload_linker)) {
 LldArgs.push_back(Arg->getValue(1));
 Arg->claim();


Index: clang/test/Driver/hip-toolchain-rdc-static-lib.hip
===
--- clang/test/Driver/hip-toolchain-rdc-static-lib.hip
+++ clang/test/Driver/hip-toolchain-rdc-static-lib.hip
@@ -80,6 +80,7 @@
 // CHECK-NOT: ".*llc"
 // CHECK: [[LLD]] {{.*}} "-plugin-opt=-amdgpu-internalize-symbols"
 // CHECK-SAME: "-plugin-opt=mcpu=gfx900"
+// CHECK-SAME: "--whole-archive"
 // CHECK-SAME: "-o" "[[IMG_DEV2:.*out]]" [[A_BC2]] [[B_BC2]]
 
 // combine images generated into hip fat binary object
Index: clang/test/Driver/hip-toolchain-rdc-separate.hip
===
--- clang/test/Driver/hip-toolchain-rdc-separate.hip
+++ clang/test/Driver/hip-toolchain-rdc-separate.hip
@@ -126,6 +126,7 @@
 // LINK-NOT: ".*llc"
 // LINK: {{".*lld.*"}} {{.*}} "-plugin-opt=-amdgpu-internalize-symbols"
 // LINK-SAME: "-plugin-opt=mcpu=gfx803"
+// LINK-SAME: "--whole-archive"
 // LLD-TMP-SAME: "-o" "[[IMG_DEV1:.*.out]]"
 // LLD-FIN-SAME: "-o" "[[IMG_DEV1:a.out-.*gfx803]]"
 // LINK-SAME "[[A_BC1]]" "[[B_BC1]]"
@@ -135,10 +136,12 @@
 // LINK-NOT: ".*llc"
 // LINK: {{".*lld.*"}} {{.*}} "-plugin-opt=-amdgpu-internalize-symbols"
 // LINK-SAME: "-plugin-opt=mcpu=gfx900"
+// LINK-SAME: "--whole-archive"
 // LLD-TMP-SAME: "-o" "[[IMG_DEV2:.*.out]]"
 // LLD-FIN-SAME: "-o" "[[IMG_DEV1:a.out-.*gfx900]]"
 // LINK-SAME "[[A_BC2]]" "[[B_BC2]]"
 
+
 // LINK-BUNDLE: [[BUNDLER:".*clang-offload-bundler"]] "-type=o"
 // LINK-BUNDLE-SAME: "-targets={{.*}},hipv4-amdgcn-amd-amdhsa--gfx803,hipv4-amdgcn-amd-amdhsa--gfx900"
 // LINK-BUNDLE-SAME: "-input={{.*}}" "-input=[[IMG_DEV1]]"

[PATCH] D152207: [HIP] Instruct lld to go through all archives

2023-06-06 Thread Yaxun Liu via Phabricator via cfe-commits 
yaxunl added inline comments.



Comment at: clang/lib/Driver/ToolChains/HIPAMD.cpp:168
+  // the linker to go through every library to look for kernel functions
+  LldArgs.push_back("--whole-archive");
   auto TargetID = Args.getLastArgValue(options::OPT_mcpu_EQ);

Better move this before line 155 to allow users to override this option.

Please add a comment that the device linker will not try to resolve a device 
variable or kernel that is referenced by a host object in the archive which in 
turn is referenced by a host object not in the archive, therefore 
--whole-archive is needed.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D152207/new/

https://reviews.llvm.org/D152207

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D152207: [HIP] Instruct lld to go through all archives

2023-06-05 Thread Siu Chi Chan via Phabricator via cfe-commits 
scchan created this revision.
Herald added a subscriber: yaxunl.
Herald added a project: All.
scchan requested review of this revision.
Herald added subscribers: cfe-commits, MaskRay.
Herald added a project: clang.

Add the --whole-archive flag when linking HIP programs to instruct lld
to go through every archive library to link in all the kernel functions
(entry pointers to the GPU program); otherwise, lld may skip some
library files if there are no more symbols that need to be resolved.

Change-Id: Ic7ea61ec3e6925f80a63d04654ac72177ffb27c8


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D152207

Files:
  clang/lib/Driver/ToolChains/HIPAMD.cpp
  clang/test/Driver/hip-toolchain-rdc-separate.hip
  clang/test/Driver/hip-toolchain-rdc-static-lib.hip


Index: clang/test/Driver/hip-toolchain-rdc-static-lib.hip
===
--- clang/test/Driver/hip-toolchain-rdc-static-lib.hip
+++ clang/test/Driver/hip-toolchain-rdc-static-lib.hip
@@ -81,6 +81,7 @@
 // CHECK: [[LLD]] {{.*}} "-plugin-opt=-amdgpu-internalize-symbols"
 // CHECK-SAME: "-plugin-opt=mcpu=gfx900"
 // CHECK-SAME: "-o" "[[IMG_DEV2:.*out]]" [[A_BC2]] [[B_BC2]]
+// CHECK-SAME: "--whole-archive"
 
 // combine images generated into hip fat binary object
 // CHECK: [[BUNDLER:".*clang-offload-bundler"]] "-type=o"
Index: clang/test/Driver/hip-toolchain-rdc-separate.hip
===
--- clang/test/Driver/hip-toolchain-rdc-separate.hip
+++ clang/test/Driver/hip-toolchain-rdc-separate.hip
@@ -129,6 +129,7 @@
 // LLD-TMP-SAME: "-o" "[[IMG_DEV1:.*.out]]"
 // LLD-FIN-SAME: "-o" "[[IMG_DEV1:a.out-.*gfx803]]"
 // LINK-SAME "[[A_BC1]]" "[[B_BC1]]"
+// LINK-SAME: "--whole-archive"
 
 // LINK-NOT: "*.llvm-link"
 // LINK-NOT: ".*opt"
@@ -138,6 +139,7 @@
 // LLD-TMP-SAME: "-o" "[[IMG_DEV2:.*.out]]"
 // LLD-FIN-SAME: "-o" "[[IMG_DEV1:a.out-.*gfx900]]"
 // LINK-SAME "[[A_BC2]]" "[[B_BC2]]"
+// LINK-SAME: "--whole-archive"
 
 // LINK-BUNDLE: [[BUNDLER:".*clang-offload-bundler"]] "-type=o"
 // LINK-BUNDLE-SAME: 
"-targets={{.*}},hipv4-amdgcn-amd-amdhsa--gfx803,hipv4-amdgcn-amd-amdhsa--gfx900"
Index: clang/lib/Driver/ToolChains/HIPAMD.cpp
===
--- clang/lib/Driver/ToolChains/HIPAMD.cpp
+++ clang/lib/Driver/ToolChains/HIPAMD.cpp
@@ -163,6 +163,9 @@
 
   // Look for archive of bundled bitcode in arguments, and add temporary files
   // for the extracted archive of bitcode to inputs.
+  // The archive libraries have to be linked with --whole-archive to instruct
+  // the linker to go through every library to look for kernel functions
+  LldArgs.push_back("--whole-archive");
   auto TargetID = Args.getLastArgValue(options::OPT_mcpu_EQ);
   AddStaticDeviceLibsLinking(C, *this, JA, Inputs, Args, LldArgs, "amdgcn",
  TargetID,


Index: clang/test/Driver/hip-toolchain-rdc-static-lib.hip
===
--- clang/test/Driver/hip-toolchain-rdc-static-lib.hip
+++ clang/test/Driver/hip-toolchain-rdc-static-lib.hip
@@ -81,6 +81,7 @@
 // CHECK: [[LLD]] {{.*}} "-plugin-opt=-amdgpu-internalize-symbols"
 // CHECK-SAME: "-plugin-opt=mcpu=gfx900"
 // CHECK-SAME: "-o" "[[IMG_DEV2:.*out]]" [[A_BC2]] [[B_BC2]]
+// CHECK-SAME: "--whole-archive"
 
 // combine images generated into hip fat binary object
 // CHECK: [[BUNDLER:".*clang-offload-bundler"]] "-type=o"
Index: clang/test/Driver/hip-toolchain-rdc-separate.hip
===
--- clang/test/Driver/hip-toolchain-rdc-separate.hip
+++ clang/test/Driver/hip-toolchain-rdc-separate.hip
@@ -129,6 +129,7 @@
 // LLD-TMP-SAME: "-o" "[[IMG_DEV1:.*.out]]"
 // LLD-FIN-SAME: "-o" "[[IMG_DEV1:a.out-.*gfx803]]"
 // LINK-SAME "[[A_BC1]]" "[[B_BC1]]"
+// LINK-SAME: "--whole-archive"
 
 // LINK-NOT: "*.llvm-link"
 // LINK-NOT: ".*opt"
@@ -138,6 +139,7 @@
 // LLD-TMP-SAME: "-o" "[[IMG_DEV2:.*.out]]"
 // LLD-FIN-SAME: "-o" "[[IMG_DEV1:a.out-.*gfx900]]"
 // LINK-SAME "[[A_BC2]]" "[[B_BC2]]"
+// LINK-SAME: "--whole-archive"
 
 // LINK-BUNDLE: [[BUNDLER:".*clang-offload-bundler"]] "-type=o"
 // LINK-BUNDLE-SAME: "-targets={{.*}},hipv4-amdgcn-amd-amdhsa--gfx803,hipv4-amdgcn-amd-amdhsa--gfx900"
Index: clang/lib/Driver/ToolChains/HIPAMD.cpp
===
--- clang/lib/Driver/ToolChains/HIPAMD.cpp
+++ clang/lib/Driver/ToolChains/HIPAMD.cpp
@@ -163,6 +163,9 @@
 
   // Look for archive of bundled bitcode in arguments, and add temporary files
   // for the extracted archive of bitcode to inputs.
+  // The archive libraries have to be linked with --whole-archive to instruct
+  // the linker to go through every library to look for kernel functions
+  LldArgs.push_back("--whole-archive");
   auto TargetID = Args.getLastArgValue(options::OPT_mcpu_EQ);
   AddStaticDeviceLibsLinking(C, *this, JA, Inputs, Args,