[AMDGPU] Prepend --no-undefined option for linker instead of append

[lamb-j]

Summary

Previously, for linking in amdgpu contexts, the --no-undefined was appended to the options passed to lld,
overriding any user-supplied options via "-Wl," or "-Xlinker". We now prepend --no-undefined so that
the user options are respected.

[jhuber6]

Remember to clang format.

clang/lib/Driver/ToolChains/AMDGPU.cpp

566–577

A little more concisely.

[yaxunl]

The -Wl and -Xlinker options are intended for the host linker and we intentionally do not pass them to the device linker.

If users want to pass options to the device linker, they need to use -Xoffload-linker.

There are multiple options affecting the handling of unresolved symbols. I think the proper way is to use --no-undefined as the default, which is always passed before those options from -Xoffload-linker so that the latter can override the former.

I believe the driver already passes -Xoffload-linker options to amdgpu::Linker::ConstructJob by Args. I think we probably only need to move all the default options (line 563-566) to 554 so that they can be overridden.

[jhuber6]

In D158582#4610023, @yaxunl wrote:

The -Wl and -Xlinker options are intended for the host linker and we intentionally do not pass them to the device linker.

If users want to pass options to the device linker, they need to use -Xoffload-linker.

There are multiple options affecting the handling of unresolved symbols. I think the proper way is to use --no-undefined as the default, which is always passed before those options from -Xoffload-linker so that the latter can override the former.

I believe the driver already passes -Xoffload-linker options to amdgpu::Linker::ConstructJob by Args. I think we probably only need to move all the default options (line 563-566) to 554 so that they can be overridden.

Yeah, the original problem was not being able to overload the defaults. This fundamentally is just because the -Wl options are being added in AddLinkerInputs before the defaults. Surprised I didn't catch that, thanks.

[lamb-j]

Is void amdgpu::Linker::ConstructJob() constructing a job for the host linker or the device linker? Or both?

The -Wl and -Xlinker options are intended for the host linker and we intentionally do not pass them to the device linker.

In the example I'm testing, both -Wl and -Xlinker result in a forwarded option to the lld invocation (as shown in the behavior of the lit tests). However, when testing with -Xoffload-linker , I just get the following:

warning: argument unused during compilation: '-Xoffload-linker --unresolved-symbols=ignore-all' [-Wunused-command-line-argument]

[yaxunl]

clang/test/Driver/amdgpu-toolchain-opencl.cl

34

we need to make sure "--unresolved-symbols=ignore-all" is after "--no-undefined" . same as below

[yaxunl]

LGTM. Thanks.

clang/lib/Driver/ToolChains/AMDGPU.cpp

 void amdgpu::Linker::ConstructJob(Compilation &C, const JobAction &JA,
                                   const InputInfo &Output,
                                   const InputInfoList &Inputs,
                                   const ArgList &Args,
                                   const char *LinkingOutput) const {
 
   std::string Linker = getToolChain().GetProgramPath(getShortName());
   ArgStringList CmdArgs;
+  CmdArgs.push_back("--no-undefined");
+  CmdArgs.push_back("-shared");
+
   addLinkerCompressDebugSectionsOption(getToolChain(), Args, CmdArgs);
   Args.AddAllArgs(CmdArgs, options::OPT_L);
   AddLinkerInputs(getToolChain(), Inputs, Args, CmdArgs, JA);
   if (C.getDriver().isUsingLTO())
     addLTOOptions(getToolChain(), Args, CmdArgs, Output, Inputs[0],
                   C.getDriver().getLTOMode() == LTOK_Thin);
   else if (Args.hasArg(options::OPT_mcpu_EQ))
     CmdArgs.push_back(Args.MakeArgString(
         "-plugin-opt=mcpu=" + Args.getLastArgValue(options::OPT_mcpu_EQ)));
-  CmdArgs.push_back("--no-undefined");
-  CmdArgs.push_back("-shared");
   CmdArgs.push_back("-o");
   CmdArgs.push_back(Output.getFilename());
   C.addCommand(std::make_unique<Command>(
       JA, *this, ResponseFileSupport::AtFileCurCP(), Args.MakeArgString(Linker),
       CmdArgs, Inputs, Output));
 }
 
 void amdgpu::getAMDGPUTargetFeatures(const Driver &D,
                                      const llvm::Triple &Triple,
                                      const llvm::opt::ArgList &Args,
                                      std::vector<StringRef> &Features) {
   // Add target ID features to -target-feature options. No diagnostics should
   // be emitted here since invalid target ID is diagnosed at other places.
   StringRef TargetID = Args.getLastArgValue(options::OPT_mcpu_EQ);
   if (!TargetID.empty()) {
     llvm::StringMap<bool> FeatureMap;
     auto OptionalGpuArch = parseTargetID(Triple, TargetID, &FeatureMap);
     if (OptionalGpuArch) {
       StringRef GpuArch = *OptionalGpuArch;
       // Iterate through all possible target ID features for the given GPU.

clang/test/Driver/amdgpu-toolchain-opencl.cl

 // CHECK_Ofast: "-cc1"{{.*}} "-Ofast"
 // CHECK_O_DEFAULT: "-cc1"{{.*}} "-O3"
 
 // CHK-INC: "-cc1" {{.*}}"-finclude-default-header" "-fdeclare-opencl-builtins" {{.*}}"-x" "cl"
 // CHK-INC-NOT: "-cc1" {{.*}}"-finclude-default-header" "-fdeclare-opencl-builtins" {{.*}}"-x" "cpp-output"
 
 // RUN: %clang -### --target=amdgcn-amd-amdhsa-opencl -x cl -mcpu=fiji -nogpulib %s 2>&1 | FileCheck -check-prefix=CHK-LINK %s
 // CHK-LINK: ld.lld{{.*}} "--no-undefined" "-shared"
+
+// RUN: %clang -### --target=amdgcn-amd-amdhsa-opencl -Wl,--unresolved-symbols=ignore-all -x cl -mcpu=fiji -nogpulib %s 2>&1 | FileCheck -check-prefix=CHK-LINK_UR %s
+// RUN: %clang -### --target=amdgcn-amd-amdhsa-opencl -Xlinker --unresolved-symbols=ignore-all -x cl -mcpu=fiji -nogpulib %s 2>&1 | FileCheck -check-prefix=CHK-LINK_UR %s
+// CHK-LINK_UR: ld.lld{{.*}} "--no-undefined"{{.*}} "--unresolved-symbols=ignore-all"

clang/test/Driver/amdgpu-toolchain.c

 // RUN: %clang -### --target=amdgcn--amdhsa -x assembler -mcpu=kaveri %s 2>&1 | FileCheck -check-prefix=AS_LINK %s
 // RUN: %clang -### -g --target=amdgcn--amdhsa -mcpu=kaveri -nogpulib %s 2>&1 | FileCheck -check-prefix=DWARF_VER %s
 // RUN: %clang -### --target=amdgcn-amd-amdpal -x assembler -mcpu=kaveri %s 2>&1 | FileCheck -check-prefix=AS_LINK %s
 // RUN: %clang -### -g --target=amdgcn-amd-amdpal -mcpu=kaveri %s 2>&1 | FileCheck -check-prefix=DWARF_VER %s
 // RUN: %clang -### --target=amdgcn-mesa-mesa3d -x assembler -mcpu=kaveri %s 2>&1 | FileCheck -check-prefix=AS_LINK %s
 // RUN: %clang -### -g --target=amdgcn-mesa-mesa3d -mcpu=kaveri %s 2>&1 | FileCheck -check-prefix=DWARF_VER %s
 
 // AS_LINK: "-cc1as"
 // AS_LINK: ld.lld{{.*}} "--no-undefined" "-shared"
 
 // DWARF_VER: "-dwarf-version=5"
 
+// RUN: %clang -### --target=amdgcn--amdhsa -x assembler \
+// RUN:  -Wl,--unresolved-symbols=ignore-all %s 2>&1 | FileCheck -check-prefix=AS_LINK_UR %s
+// RUN: %clang -### --target=amdgcn--amdhsa -x assembler \
+// RUN:  -Xlinker --unresolved-symbols=ignore-all %s 2>&1 | FileCheck -check-prefix=AS_LINK_UR %s
+
+// AS_LINK_UR: "-cc1as"
+// AS_LINK_UR: ld.lld{{.*}} "--no-undefined"{{.*}} "--unresolved-symbols=ignore-all"
+
 // RUN: %clang -### --target=amdgcn-amd-amdhsa -mcpu=gfx906 -nogpulib \
 // RUN:   -L. -flto -fconvergent-functions %s 2>&1 | FileCheck -check-prefixes=LTO,MCPU %s
 // RUN: %clang -### --target=amdgcn-amd-amdhsa -mcpu=gfx906 -nogpulib \
 // RUN:   -L. -fconvergent-functions %s 2>&1 | FileCheck -check-prefix=MCPU %s
 // LTO: clang{{.*}} "-flto=full"{{.*}}"-fconvergent-functions"
 // MCPU: ld.lld{{.*}}"-L."{{.*}}"-plugin-opt=mcpu=gfx906"