[HIP] Fix -mllvm option for device lld linker

[yaxunl]

Summary

currently clang passes -mllvm options to the device lld linker plugin
when compiling HIP. This is against default clang behavior
which is only passing -mllvm options to linker plugin specified through -Wl
options. This patch lets clang only pass -Xoffload-linker -mllvm= options
to device lld linker plugin.

Fixes: https://github.com/llvm/llvm-project/issues/63604

[jhuber6]

LG, thanks.

[arsenm]

clang/lib/Driver/ToolChains/HIPAMD.cpp

164

StringRef Prefix("-mllvm=") and then use the length instead of strlen

[jhuber6]

clang/lib/Driver/ToolChains/HIPAMD.cpp

164

You could probably also do ArgVal.split("-mllvm=").second and push back first or second depending on whether or not second is empty.

[yaxunl]

clang/lib/Driver/ToolChains/HIPAMD.cpp

164

will use ArgVal.split

clang/lib/Driver/ToolChains/HIPAMD.cpp

if (!Features.empty())
     LldArgs.push_back(Args.MakeArgString(MAttrString));
 
   // ToDo: Remove this option after AMDGPU backend supports ISA-level linking.
   // Since AMDGPU backend currently does not support ISA-level linking, all
   // called functions need to be imported.
   if (IsThinLTO)
     LldArgs.push_back(Args.MakeArgString("-plugin-opt=-force-import-all"));
 
-  for (const Arg *A : Args.filtered(options::OPT_mllvm)) {
-    LldArgs.push_back(
-        Args.MakeArgString(Twine("-plugin-opt=") + A->getValue(0)));
-  }
-
   if (C.getDriver().isSaveTempsEnabled())
     LldArgs.push_back("-save-temps");
 
   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));
+    StringRef ArgVal = Arg->getValue(1);
+    auto SplitArg = ArgVal.split("-mllvm=");
+    if (!SplitArg.second.empty()) {
+      LldArgs.push_back(
+          Args.MakeArgString(Twine("-plugin-opt=") + SplitArg.second));
+    } else {
+      LldArgs.push_back(Args.MakeArgString(ArgVal));
+    }
     Arg->claim();
   }
 
   LldArgs.append({"-o", Output.getFilename()});
   for (auto Input : Inputs)
     LldArgs.push_back(Input.getFilename());
 
   // Look for archive of bundled bitcode in arguments, and add temporary files

clang/test/Driver/hip-toolchain-mllvm.hip

 // REQUIRES: x86-registered-target
 // REQUIRES: amdgpu-registered-target
 
+// Check only -Xoffload-linker -mllvm=* options are passed
+// to device lld linker.
+// -mllvm options are passed to clang only.
+
 // RUN: %clang -### --target=x86_64-linux-gnu \
 // RUN:   --cuda-gpu-arch=gfx803 --cuda-gpu-arch=gfx900 \
-// RUN:   -mllvm -amdgpu-function-calls=0 \
+// RUN:   -mllvm -unroll-count=10 \
+// RUN:   -Xoffload-linker -mllvm=-inline-threshold=100 \
 // RUN:   %s 2>&1 | FileCheck %s
 
 // RUN: %clang -### --target=x86_64-linux-gnu \
 // RUN:   --cuda-gpu-arch=gfx803 --cuda-gpu-arch=gfx900 \
-// RUN:   -fgpu-rdc -mllvm -amdgpu-function-calls=0 \
-// RUN:   %s 2>&1 | FileCheck -check-prefixes=CHECK,RDC %s
+// RUN:   -mllvm -unroll-count=10 \
+// RUN:   -Xoffload-linker -mllvm=-inline-threshold=100 \
+// RUN:   %s 2>&1 | FileCheck -check-prefix=NEG %s
+
+// RUN: %clang -### --target=x86_64-linux-gnu \
+// RUN:   --cuda-gpu-arch=gfx803 --cuda-gpu-arch=gfx900 \
+// RUN:   -fgpu-rdc -mllvm -unroll-count=10 \
+// RUN:   -Xoffload-linker -mllvm=-inline-threshold=100 \
+// RUN:   %s 2>&1 | FileCheck %s
+
+// RUN: %clang -### --target=x86_64-linux-gnu \
+// RUN:   --cuda-gpu-arch=gfx803 --cuda-gpu-arch=gfx900 \
+// RUN:   -fgpu-rdc -mllvm -unroll-count=10 \
+// RUN:   -Xoffload-linker -mllvm=-inline-threshold=100 \
+// RUN:   %s 2>&1 | FileCheck -check-prefix=NEG %s
 
 // CHECK: [[CLANG:".*clang.*"]] "-cc1" "-triple" "amdgcn-amd-amdhsa"
 // CHECK-SAME: "-aux-triple" "x86_64-unknown-linux-gnu"
 // CHECK-SAME: {{.*}} "-target-cpu" "gfx803"
-// CHECK-SAME: {{.*}} "-mllvm" "-amdgpu-function-calls=0" {{.*}}
-
-// CHECK-NOT: {{".*opt"}}
-// CHECK-NOT: {{".*llc"}}
-// RDC: [[LLD:".*lld.*"]] {{.*}} "-plugin-opt=-amdgpu-function-calls=0"
+// CHECK-SAME: {{.*}} "-mllvm" "-unroll-count=10" {{.*}}
+// CHECK: [[LLD:".*lld.*"]] {{.*}}"-m" "elf64_amdgpu"{{.*}} "-plugin-opt=-inline-threshold=100"
 
 // CHECK: [[CLANG]] "-cc1" "-triple" "amdgcn-amd-amdhsa"
 // CHECK-SAME: "-aux-triple" "x86_64-unknown-linux-gnu"
 // CHECK-SAME: {{.*}} "-target-cpu" "gfx900"
-// CHECK-SAME: {{.*}} "-mllvm" "-amdgpu-function-calls=0" {{.*}}
+// CHECK-SAME: {{.*}} "-mllvm" "-unroll-count=10" {{.*}}
+// CHECK: [[LLD:".*lld.*"]] {{.*}} "-plugin-opt=-inline-threshold=100"
 
-// CHECK-NOT: {{".*opt"}}
-// CHECK-NOT: {{".*llc"}}
-// RDC: [[LLD:".*lld.*"]] {{.*}} "-plugin-opt=-amdgpu-function-calls=0"
+// NEG-NOT: {{".*opt"}}
+// NEG-NOT: {{".*llc"}}
+// NEG-NOT: "-plugin-opt=-unroll-count=10"
+// NEG-NOT: "-m" "elf_x86_64"{{.*}} "-plugin-opt=-inline-threshold=100"