[SYCL][clang-linker-wrapper] Update --bitcode-library option for SYCL (#20331)

This PR updates the `--bitcode-library` option in `clang-linker-wrapper`
to link bitcode device libraries that are used in NVIDIA and AMD GPU
targets in SYCL offloading. This option had some previous behavior, but
it was unused, so I deleted it and repurposed it for SYCL.

`SYCL::getDeviceLibraries()` returns both fat objects and bitcode device
libraries mixed together, while the `-sycl-device-libraries` option only
handles fat objects. This was causing `clang-linker-wrapper` to not link
the `libdevice` for nvptx/AMD targets.

This PR also removes the `-sycl-nvptx-device-libraries` option, which
seemed to be doing the same thing but only for nvptx targets.

Author: jzc

clang/lib/Driver/ToolChains/Clang.cpp

@@ -11222,43 +11222,6 @@ void LinkerWrapper::ConstructJob(Compilation &C, const JobAction &JA,
     if (Args.hasArg(options::OPT_fsycl_link_EQ))
       CmdArgs.push_back(Args.MakeArgString("--sycl-device-link"));
 
-    // -sycl-device-libraries=<comma separated list> contains all of the SYCL
-    // device specific libraries that are needed. This generic list will be
-    // populated with device binaries for all target triples in the current
-    // compilation flow.
-
-    // Create a comma separated list to pass along to the linker wrapper.
-    SmallString<256> LibList;
-    llvm::Triple TargetTriple;
-    auto ToolChainRange = C.getOffloadToolChains<Action::OFK_SYCL>();
-    for (auto &I :
-         llvm::make_range(ToolChainRange.first, ToolChainRange.second)) {
-      const ToolChain *TC = I.second;
-      // Note: For AMD targets, we do not pass any SYCL device libraries.
-      if (TC->getTriple().isSPIROrSPIRV() || TC->getTriple().isNVPTX()) {
-        TargetTriple = TC->getTriple();
-        SmallVector<std::string, 8> SYCLDeviceLibs;
-        bool IsSPIR = TargetTriple.isSPIROrSPIRV();
-        bool IsSpirvAOT = TargetTriple.isSPIRAOT();
-        bool UseJitLink =
-            IsSPIR &&
-            Args.hasFlag(options::OPT_fsycl_device_lib_jit_link,
-                         options::OPT_fno_sycl_device_lib_jit_link, false);
-        bool UseAOTLink = IsSPIR && (IsSpirvAOT || !UseJitLink);
-        SYCLDeviceLibs = SYCL::getDeviceLibraries(C, TargetTriple, UseAOTLink);
-        for (const auto &AddLib : SYCLDeviceLibs) {
-          if (LibList.size() > 0)
-            LibList += ",";
-          LibList += AddLib;
-        }
-      }
-    }
-    // -sycl-device-libraries=<libs> provides a comma separate list of
-    // libraries to add to the device linking step.
-    if (LibList.size())
-      CmdArgs.push_back(
-          Args.MakeArgString(Twine("-sycl-device-libraries=") + LibList));
-
     // -sycl-device-library-location=<dir> provides the location in which the
     // SYCL device libraries can be found.
     SmallString<128> DeviceLibDir(D.Dir);
@@ -11283,6 +11246,68 @@ void LinkerWrapper::ConstructJob(Compilation &C, const JobAction &JA,
         break;
       }
     }
+
+    // -sycl-device-libraries=<comma separated list> contains a list of
+    // file names for fat object files that contain SYCL device library bitcode
+    // necessary for SYCL offloading that will be linked to the user's device
+    // code. clang-linker-wrapper uses the value provided to
+    // -sycl-device-library-location=<dir> to construct the full paths of the
+    // device libraries.
+
+    // On the other hand, --bitcode-library=<triple>=<path to bc file> specifies
+    // one bitcode library to link in for a specific triple. Additionally, the
+    // path is *not* relative to the -sycl-device-library-location - the full
+    // path must be provided.
+    SmallString<256> LibList;
+    SmallVector<std::string, 4> BCLibList;
+
+    auto appendToList = [](SmallString<256> &List, const Twine &Arg) {
+      if (List.size() > 0)
+        List += ",";
+      List += Arg.str();
+    };
+
+    auto ToolChainRange = C.getOffloadToolChains<Action::OFK_SYCL>();
+    for (const auto &[Kind, TC] :
+         llvm::make_range(ToolChainRange.first, ToolChainRange.second)) {
+      llvm::Triple TargetTriple = TC->getTriple();
+      bool IsSPIR = TargetTriple.isSPIROrSPIRV();
+      bool IsSpirAOT = TargetTriple.isSPIRAOT();
+      bool UseJitLink =
+          IsSPIR &&
+          Args.hasFlag(options::OPT_fsycl_device_lib_jit_link,
+                       options::OPT_fno_sycl_device_lib_jit_link, false);
+      bool UseAOTLink = IsSPIR && (IsSpirAOT || !UseJitLink);
+      SmallVector<std::string, 8> SYCLDeviceLibs =
+          SYCL::getDeviceLibraries(C, TargetTriple, UseAOTLink);
+      for (const auto &AddLib : SYCLDeviceLibs) {
+        if (llvm::sys::path::extension(AddLib) == ".bc") {
+          SmallString<256> LibPath(DeviceLibDir);
+          llvm::sys::path::append(LibPath, AddLib);
+          BCLibList.push_back(
+              (Twine(TC->getTriple().str()) + "=" + LibPath).str());
+          continue;
+        }
+
+        appendToList(LibList, AddLib);
+      }
+
+      if (TC->getTriple().isNVPTX())
+        if (const char *LibSpirvFile = SYCLInstallation.findLibspirvPath(
+                TC->getTriple(), Args, *TC->getAuxTriple()))
+          BCLibList.push_back(
+              (Twine(TC->getTriple().str()) + "=" + LibSpirvFile).str());
+    }
+
+    if (LibList.size())
+      CmdArgs.push_back(
+          Args.MakeArgString(Twine("-sycl-device-libraries=") + LibList));
+
+    if (BCLibList.size())
+      for (const std::string &Lib : BCLibList)
+        CmdArgs.push_back(
+            Args.MakeArgString(Twine("--bitcode-library=") + Lib));
+
     CmdArgs.push_back(Args.MakeArgString(
         Twine("-sycl-device-library-location=") + DeviceLibDir));
 

clang/test/Driver/sycl-bc-device-libraries.cpp

@@ -0,0 +1,79 @@
+/// Test that SYCL bitcode device libraries are properly separated for NVIDIA and AMD targets.
+
+/// Check devicelib and libspirv are linked for nvptx.
+// RUN: %clang -### -fsycl --offload-new-driver \
+// RUN:   -fsycl-targets=nvptx64-nvidia-cuda \
+// RUN:   --cuda-path=%S/Inputs/CUDA_102/usr/local/cuda \
+// RUN:   %s 2>&1 | FileCheck -check-prefix=CHECK-NVPTX-BC %s
+
+// RUN: %clang_cl -### -fsycl --offload-new-driver \
+// RUN:   -fsycl-targets=nvptx64-nvidia-cuda \
+// RUN:   --cuda-path=%S/Inputs/CUDA_102/usr/local/cuda \
+// RUN:   %s 2>&1 | FileCheck -check-prefix=CHECK-NVPTX-BC %s
+
+// CHECK-NVPTX-BC: clang-linker-wrapper
+// CHECK-NVPTX-BC-SAME: "--bitcode-library=nvptx64-nvidia-cuda={{.*}}devicelib-nvptx64-nvidia-cuda.bc" "--bitcode-library=nvptx64-nvidia-cuda={{.*}}libspirv-nvptx64-nvidia-cuda.bc"
+
+/// Check devicelib is linked for amdgcn.
+// RUN: %clang -### -fsycl --offload-new-driver \
+// RUN:   -fsycl-targets=amdgcn-amd-amdhsa \
+// RUN:   -Xsycl-target-backend=amdgcn-amd-amdhsa --offload-arch=gfx900 \
+// RUN:   --rocm-path=%S/Inputs/rocm \
+// RUN:   %s 2>&1 | FileCheck -check-prefix=CHECK-AMD-BC %s
+
+// RUN: %clang_cl -### -fsycl --offload-new-driver \
+// RUN:   -fsycl-targets=amdgcn-amd-amdhsa \
+// RUN:   -Xsycl-target-backend=amdgcn-amd-amdhsa --offload-arch=gfx900 \
+// RUN:   --rocm-path=%S/Inputs/rocm \
+// RUN:   %s 2>&1 | FileCheck -check-prefix=CHECK-AMD-BC %s
+
+// CHECK-AMD-BC: clang-linker-wrapper
+// CHECK-AMD-BC-SAME: "--bitcode-library=amdgcn-amd-amdhsa={{.*}}devicelib-amdgcn-amd-amdhsa.bc"
+
+/// Check linking with multiple targets.
+// RUN: %clang -### -fsycl --offload-new-driver \
+// RUN:   -fsycl-targets=amdgcn-amd-amdhsa,nvptx64-nvidia-cuda \
+// RUN:   -Xsycl-target-backend=amdgcn-amd-amdhsa --offload-arch=gfx900 \
+// RUN:   --cuda-path=%S/Inputs/CUDA_102/usr/local/cuda \
+// RUN:   --rocm-path=%S/Inputs/rocm \
+// RUN:   %s 2>&1 | FileCheck -check-prefix=CHECK-MULTI-TARGET %s
+
+// RUN: %clang_cl -### -fsycl --offload-new-driver \
+// RUN:   -fsycl-targets=amdgcn-amd-amdhsa,nvptx64-nvidia-cuda \
+// RUN:   -Xsycl-target-backend=amdgcn-amd-amdhsa --offload-arch=gfx900 \
+// RUN:   --cuda-path=%S/Inputs/CUDA_102/usr/local/cuda \
+// RUN:   --rocm-path=%S/Inputs/rocm \
+// RUN:   %s 2>&1 | FileCheck -check-prefix=CHECK-MULTI-TARGET %s
+
+// CHECK-MULTI-TARGET: clang-linker-wrapper
+// CHECK-MULTI-TARGET-SAME: "--bitcode-library=amdgcn-amd-amdhsa={{.*}}devicelib-amdgcn-amd-amdhsa.bc" "--bitcode-library=nvptx64-nvidia-cuda={{.*}}devicelib-nvptx64-nvidia-cuda.bc" "--bitcode-library=nvptx64-nvidia-cuda={{.*}}libspirv-nvptx64-nvidia-cuda.bc"
+
+/// Test --bitcode-library with nvptx dummy libraries.
+// RUN: %clang -cc1 %s -triple nvptx64-nvidia-cuda -emit-llvm-bc -o %t.nvptx.devicelib.bc
+// RUN: %clang -cc1 %s -triple nvptx64-nvidia-cuda -emit-llvm-bc -o %t.nvptx.libspirv.bc
+// RUN: %clangxx -fsycl -fsycl-targets=nvptx64-nvidia-cuda --offload-new-driver -c %s -o %t.nvptx.o -nocudalib
+// RUN: clang-linker-wrapper --bitcode-library=nvptx64-nvidia-cuda=%t.nvptx.devicelib.bc --bitcode-library=nvptx64-nvidia-cuda=%t.nvptx.libspirv.bc \
+// RUN:   --host-triple=x86_64-unknown-linux-gnu --dry-run \
+// RUN:   --linker-path=/usr/bin/ld %t.nvptx.o -o a.out 2>&1 | FileCheck -check-prefix=CHECK-WRAPPER-NVPTX %s
+
+// CHECK-WRAPPER-NVPTX: llvm-link{{.*}} {{.*}}.nvptx.devicelib.bc {{.*}}.nvptx.libspirv.bc
+
+/// Test --bitcode-library with amdgcn dummy library.
+// RUN: %clang -cc1 %s -triple amdgcn-amd-amdhsa -emit-llvm-bc -o %t.amd.devicelib.bc
+// RUN: %clangxx -fsycl -fsycl-targets=amdgcn-amd-amdhsa -Xsycl-target-backend=amdgcn-amd-amdhsa --offload-arch=gfx900 --offload-new-driver -c %s -o %t.amd.o -nogpulib
+// RUN: clang-linker-wrapper --bitcode-library=amdgcn-amd-amdhsa=%t.amd.devicelib.bc \
+// RUN:   --host-triple=x86_64-unknown-linux-gnu --dry-run \
+// RUN:   --linker-path=/usr/bin/ld %t.amd.o -o a.out 2>&1 | FileCheck -check-prefix=CHECK-WRAPPER-AMD %s
+
+// CHECK-WRAPPER-AMD: llvm-link{{.*}} {{.*}}.amd.devicelib.bc
+
+/// Test --bitcode-library with multi-target bc libraries.
+// RUN: %clangxx -fsycl -fsycl-targets=amdgcn-amd-amdhsa,nvptx64-nvidia-cuda \
+// RUN:   -Xsycl-target-backend=amdgcn-amd-amdhsa --offload-arch=gfx900 \
+// RUN:   --offload-new-driver -c %s -o %t.multi.o -nocudalib -nogpulib
+// RUN: clang-linker-wrapper --bitcode-library=amdgcn-amd-amdhsa=%t.amd.devicelib.bc --bitcode-library=nvptx64-nvidia-cuda=%t.nvptx.devicelib.bc --bitcode-library=nvptx64-nvidia-cuda=%t.nvptx.libspirv.bc \
+// RUN:   --host-triple=x86_64-unknown-linux-gnu --dry-run \
+// RUN:   --linker-path=/usr/bin/ld %t.multi.o -o a.out 2>&1 | FileCheck -check-prefix=CHECK-WRAPPER-MULTI %s
+
+// CHECK-WRAPPER-MULTI: llvm-link{{.*}} {{.*}}.amd.devicelib.bc
+// CHECK-WRAPPER-MULTI: llvm-link{{.*}} {{.*}}.nvptx.devicelib.bc {{.*}}.nvptx.libspirv.bc

clang/tools/clang-linker-wrapper/ClangLinkerWrapper.cpp

@@ -1432,24 +1432,17 @@ static Expected<StringRef> linkDevice(ArrayRef<StringRef> InputFiles,
           << "Compatible SYCL device library binary not found\n";
   }
 
-  // For NVPTX backend we need to also link libclc and CUDA libdevice.
-  if (Triple.isNVPTX()) {
-    if (Arg *A = Args.getLastArg(OPT_sycl_nvptx_device_lib_EQ)) {
-      if (A->getValues().size() == 0)
-        return createStringError(
-            inconvertibleErrorCode(),
-            "Number of device library files cannot be zero.");
-      for (StringRef Val : A->getValues()) {
-        SmallString<128> LibName(Val);
-        if (llvm::sys::fs::exists(LibName))
-          ExtractedDeviceLibFiles.emplace_back(std::string(LibName));
-        else
-          return createStringError(
-              inconvertibleErrorCode(),
-              std::string(LibName) +
-                  " SYCL device library file for NVPTX is not found.");
-      }
-    }
+  for (StringRef Library : Args.getAllArgValues(OPT_bitcode_library_EQ)) {
+    auto [LibraryTriple, LibraryPath] = Library.split('=');
+    if (llvm::Triple(LibraryTriple) != Triple)
+      continue;
+
+    if (!llvm::sys::fs::exists(LibraryPath))
+      return createStringError(inconvertibleErrorCode(),
+                               "The specified device library " + LibraryPath +
+                                   " does not exist.");
+
+    ExtractedDeviceLibFiles.emplace_back(LibraryPath.str());
   }
 
   // Make sure that SYCL device library files are available.
@@ -2551,13 +2544,6 @@ getDeviceInput(const ArgList &Args) {
     }
   }
 
-  for (StringRef Library : Args.getAllArgValues(OPT_bitcode_library_EQ)) {
-    auto FileOrErr = getInputBitcodeLibrary(Library);
-    if (!FileOrErr)
-      return FileOrErr.takeError();
-    InputFiles[*FileOrErr].push_back(std::move(*FileOrErr));
-  }
-
   SmallVector<SmallVector<OffloadFile>> InputsForTarget;
   for (auto &[ID, Input] : InputFiles)
     InputsForTarget.emplace_back(std::move(Input));

clang/tools/clang-linker-wrapper/LinkerWrapperOpts.td

@@ -156,9 +156,6 @@ def sycl_device_lib_EQ : CommaJoined<["--", "-"], "sycl-device-libraries=">,
 def sycl_device_library_location_EQ : Joined<["--", "-"],
   "sycl-device-library-location=">, Flags<[WrapperOnlyOption]>,
   HelpText<"Location of SYCL device library files">;
-def sycl_nvptx_device_lib_EQ : CommaJoined<["--", "-"], "sycl-nvptx-device-libraries=">,
-  Flags<[WrapperOnlyOption]>,
-  HelpText<"A comma separated list of nvptx-specific device libraries that are linked during the device link.">;
 
 // Options for SYCL backends and linker options for shared libraries.
 def sycl_backend_compile_options_EQ :