[libomptarget] Try a fallback devicertl if the preferred one is missing

[JonChesterfield]

Summary

[libomptarget] Try a fallback devicertl if the preferred one is missing

Clang may be used with a cuda version that is newer than the clang release.
This may fail in various ways, but there is also a credible chance it will
work OK, for sufficiently small version slip.

As discussed in the weekly call, this change adds a fallback path to clang
which looks for a devicertl library that doesn't have a specific cuda version
encoded in the name, specifically 'libomptarget-nvptx-unknown.bc'. It also
warns about this, and recommends the user specify a devicertl they expect to
work manually.

[JonChesterfield]

• drop whitespace from .td

[tianshilei1992]

clang/lib/Driver/ToolChains/CommonArgs.cpp

1693–1705

If you're using Twine, + is not needed.

llvm::Twine LibOmpTargetName("libomptarget-", BitcodeSuffix ,".bc");

1695

The BitcodeSuffix for NVPTX consists of three parts: nvptx, cuda_xxx, and sm_yy. We might want to have an unknown for every sm?

1697–1698

bool FoundBCLibrary = tryAppendBuiltinBitcodeGivenPaths(
        LibraryPaths, DriverArgs, CC1Args, LibOmpTargetName);

Remove the definition above?

[tianshilei1992]

clang/include/clang/Basic/DiagnosticDriverKinds.td

265

Besides, we might also need to tell users this bitcode library might not work as expected.

[JonChesterfield]

• review comments

[JonChesterfield]

clang/lib/Driver/ToolChains/CommonArgs.cpp

1647

^clang-tidy warning about a twine in a parameter list [llvm-twine-local] looks like a bug

1693–1705

This doesn't appear to be the case. Doesn't compile anyway, no matching constructor.

The discussion around the clang-tidy warning states that Twine variables shouldn't be used as local variables because they don't make copies of the arguments, so one can hit use after free. I don't agree with that, but it doesn't matter very much so have changed to std::string.

1695

Lets not go there. 'Unknown' is a reasonably strong indicator for 'this might not work'. One per sm suggests we've somehow tested whether it's going to be OK or not.

I'd prefer we stick with 'using a cuda that didn't exist when your clang shipped is a bad idea, get a newer clang or an older cuda'. This is a sketch of how we could go the other way.

[JonChesterfield]

clang/include/clang/Basic/DiagnosticDriverKinds.td

265

Could you propose preferred wording?

[JonChesterfield]

Suggestion is to resolve libomptarget-nvptx-unknown.bc to a cp of the bitcode libary built for the newest sm_xx and ptx version clang knows of.

[tianshilei1992]

clang/include/clang/Basic/DiagnosticDriverKinds.td

265

maybe the following one?

def warn_drv_omp_offload_target_missingbcruntime : Warning<"No library '%0' found in the default clang lib directory or in LIBRARY_PATH. Fall back to '%1' instead.">;

[tianshilei1992]

Please also update the test.

In D96877#2569861, @JonChesterfield wrote:

Suggestion is to resolve libomptarget-nvptx-unknown.bc to a cp of the bitcode libary built for the newest sm_xx and ptx version clang knows of.

Why a copy instead of a symlink?

[JonChesterfield]

• address review comment

[JonChesterfield]

In D96877#2569863, @tianshilei1992 wrote:

Please also update the test.

In D96877#2569861, @JonChesterfield wrote:

Suggestion is to resolve libomptarget-nvptx-unknown.bc to a cp of the bitcode libary built for the newest sm_xx and ptx version clang knows of.

Why a copy instead of a symlink?

I don't trust symlinks to survive packaging (tar or deb) or copying around the filesystem during install. Possibly showing scars from windows development there.

Tests passing locally, as expected. Do you mean add a new one that jury rigs cuda version and has a libomptarget-nvptx-unknown.bc on disk somewhere?

[jdoerfert]

Can we wrap this up and backport it, last known issue we should fix for 12.

[JonChesterfield]

If you like it, accept it. If there's more stuff to do first, please say so.

[ye-luo]

Does this patch include creating 'libomptarget-nvptx-unknown.bc'?

[tianshilei1992]

LGTM. Please also update the test(s) before commit.

[JonChesterfield]

Which tests?

Choosing a file to duplicate with the unknown.bc name is left separate. This just adds code to look for that file if the first choice failed.

[ye-luo]

Let user to copy the bc file is not feasible.
Handle this in CMake please.
libomptarget-nvptx-unknown.bc

[ye-luo]

Got it. Copy a file can be tricky. Compile one more can be easily done in cmake.

[JonChesterfield]

Is this obsolete with the change to devicertl cmake? Would prefer abandon to land if so

[tianshilei1992]

I think we might not this patch. We’re gonna not support old version of CUDA anyway.

[ye-luo]

to me this is still desired + cmake creating libomptarget-nvptx-unknown.bc as a solution for forward compatibility until a clean solution lands. I don't want repetitively asking the admin to copy files for every new CUDA toolkit installation.

[tianshilei1992]

In D96877#2578748, @ye-luo wrote:

to me this is still desired + cmake creating libomptarget-nvptx-unknown.bc as a solution for forward compatibility until a clean solution lands.

We’ll have newer version LLVM like 12.1 or 12.01 w/ a *right* solution. I don’t think we need to think that further.

[ye-luo]

In D96877#2578752, @tianshilei1992 wrote:

In D96877#2578748, @ye-luo wrote:

to me this is still desired + cmake creating libomptarget-nvptx-unknown.bc as a solution for forward compatibility until a clean solution lands.

We’ll have newer version LLVM like 12.1 or 12.01 w/ a *right* solution. I don’t think we need to think that further.

This doesn't help people who needs to run exactly 12.0. Also cannot wait for a minor release, need things to work right away when a new cuda toolkit is installed intentionally.

[tianshilei1992]

In D96877#2578756, @ye-luo wrote:

In D96877#2578752, @tianshilei1992 wrote:

In D96877#2578748, @ye-luo wrote:

to me this is still desired + cmake creating libomptarget-nvptx-unknown.bc as a solution for forward compatibility until a clean solution lands.

We’ll have newer version LLVM like 12.1 or 12.01 w/ a *right* solution. I don’t think we need to think that further.

This doesn't help people who needs to run exactly 12.0. Also cannot wait for a minor release, need things to work right away when a new cuda toolkit is installed intentionally.

First, we don’t know whether a new version of CUDA will be released during this time, especially considering the release history of CUDA, 11.3 is not that possible, and 12 will not come out so soon. Second, even if NVIDIA’s people are so brilliant and release CUDA 12 in a short time, we have a mechanism for user to work around the issue using the option libomptarget-nvptx-bc-path. This patch is just a work around, and it contains so many uncertainties, which cannot be called “forward compatibility” at all.

[JonChesterfield]

I'm going to abandon this. I'm not confident that a cuda toolkit that is newer than the compiler will work with it correctly and would prefer it take some jury rigging on the end users part to put the two together.

I get that 'works out of the box, whatever cuda is installed' would be an amazing feature to have. This patch sort of promises that without necessarily delivering on the 'works' part.

clang/include/clang/Basic/DiagnosticDriverKinds.td

 def err_drv_omp_host_ir_file_not_found : Error<
   "The provided host compiler IR file '%0' is required to generate code for OpenMP target regions but cannot be found.">;
 def err_drv_omp_host_target_not_supported : Error<
   "The target '%0' is not a supported OpenMP host target.">;
 def err_drv_expecting_fopenmp_with_fopenmp_targets : Error<
   "The option -fopenmp-targets must be used in conjunction with a -fopenmp option compatible with offloading, please use -fopenmp=libomp or -fopenmp=libiomp5.">;
 def err_drv_omp_offload_target_missingbcruntime : Error<
   "No library '%0' found in the default clang lib directory or in LIBRARY_PATH. Please use --libomptarget-%1-bc-path to specify %1 bitcode library.">;
+def warn_drv_omp_offload_target_missingbcruntime : Warning<"No library '%0' found in the default clang lib directory or in LIBRARY_PATH. Fall back to '%1' instead.">;
 def err_drv_omp_offload_target_bcruntime_not_found : Error<"Bitcode library '%0' does not exist.">;
 def warn_drv_omp_offload_target_duplicate : Warning<
   "The OpenMP offloading target '%0' is similar to target '%1' already specified - will be ignored.">,
   InGroup<OpenMPTarget>;
 def err_drv_unsupported_embed_bitcode
     : Error<"%0 is not supported with -fembed-bitcode">;
 def err_drv_bitcode_unsupported_on_toolchain : Error<
   "-fembed-bitcode is not supported on versions of iOS prior to 6.0">;

clang/lib/Driver/ToolChains/CommonArgs.cpp

if (A->getOption().matches(options::OPT_moutline)) {
       }
     } else {
       // Disable all outlining behaviour.
       addArg(Twine("-enable-machine-outliner=never"));
     }
   }
 }
 
+namespace {
+bool tryAppendBuiltinBitcode(const llvm::opt::ArgList &DriverArgs,
+                             llvm::opt::ArgStringList &CC1Args, StringRef lib) {
+  if (llvm::sys::fs::exists(lib)) {
+    CC1Args.push_back("-mlink-builtin-bitcode");
+    CC1Args.push_back(DriverArgs.MakeArgString(lib));
+    return true;
+  } else {
+    return false;
+  }
+}
+
+bool tryAppendBuiltinBitcodeGivenPaths(
+    const SmallVector<StringRef, 8> &LibraryPaths,
+    const llvm::opt::ArgList &DriverArgs, llvm::opt::ArgStringList &CC1Args,
+    StringRef lib) {
+  for (StringRef LibraryPath : LibraryPaths) {
+    SmallString<128> TargetFile(LibraryPath);
+    llvm::sys::path::append(TargetFile, lib);
+    if (tryAppendBuiltinBitcode(DriverArgs, CC1Args, TargetFile)) {
+      return true;
+    }
+  }
+  return false;
+}
+} // namespace
+
 void tools::addOpenMPDeviceRTL(const Driver &D,
                                const llvm::opt::ArgList &DriverArgs,
                                llvm::opt::ArgStringList &CC1Args,
                                StringRef BitcodeSuffix,
                                const llvm::Triple &Triple) {
   SmallVector<StringRef, 8> LibraryPaths;
   // Add user defined library paths from LIBRARY_PATH.
   llvm::Optional<std::string> LibPath =
void tools::addOpenMPDeviceRTL(const Driver &D,
   OptSpecifier LibomptargetBCPathOpt =
       Triple.isAMDGCN() ? options::OPT_libomptarget_amdgcn_bc_path_EQ
                         : options::OPT_libomptarget_nvptx_bc_path_EQ;
 
   StringRef ArchPrefix = Triple.isAMDGCN() ? "amdgcn" : "nvptx";
   // First check whether user specifies bc library
   if (const Arg *A = DriverArgs.getLastArg(LibomptargetBCPathOpt)) {
     std::string LibOmpTargetName(A->getValue());
-    if (llvm::sys::fs::exists(LibOmpTargetName)) {
-      CC1Args.push_back("-mlink-builtin-bitcode");
-      CC1Args.push_back(DriverArgs.MakeArgString(LibOmpTargetName));
-    } else {
+    if (!tryAppendBuiltinBitcode(DriverArgs, CC1Args, LibOmpTargetName)) {
       D.Diag(diag::err_drv_omp_offload_target_bcruntime_not_found)
           << LibOmpTargetName;
     }
   } else {
-    bool FoundBCLibrary = false;
-
-    std::string LibOmpTargetName =
-        "libomptarget-" + BitcodeSuffix.str() + ".bc";
-
-    for (StringRef LibraryPath : LibraryPaths) {
-      SmallString<128> LibOmpTargetFile(LibraryPath);
-      llvm::sys::path::append(LibOmpTargetFile, LibOmpTargetName);
-      if (llvm::sys::fs::exists(LibOmpTargetFile)) {
-        CC1Args.push_back("-mlink-builtin-bitcode");
-        CC1Args.push_back(DriverArgs.MakeArgString(LibOmpTargetFile));
-        FoundBCLibrary = true;
-        break;
+    std::string Root = "libomptarget-";
+    std::string LibOmpTargetName = Root + BitcodeSuffix.str() + ".bc";
+    std::string FallbackTargetName = Root + ArchPrefix.str() + "-unknown.bc";
+
+    bool FoundBCLibrary = tryAppendBuiltinBitcodeGivenPaths(
+        LibraryPaths, DriverArgs, CC1Args, LibOmpTargetName);
+
+    if (!FoundBCLibrary) {
+      FoundBCLibrary = tryAppendBuiltinBitcodeGivenPaths(
+          LibraryPaths, DriverArgs, CC1Args, FallbackTargetName);
+      if (FoundBCLibrary) {
+        D.Diag(diag::warn_drv_omp_offload_target_missingbcruntime)
+            << LibOmpTargetName << FallbackTargetName;
       }
     }
 
     if (!FoundBCLibrary)
       D.Diag(diag::err_drv_omp_offload_target_missingbcruntime)
           << LibOmpTargetName << ArchPrefix;
   }
 }