[PATCH] D117246: [OpenMP] Add support for linking AMDGPU images

Wed Jan 19 10:27:41 PST 2022

JonChesterfield added a comment.

A bit nervous about the automatic appending of host library search paths for the device link but I see that's pre-existing in nvptx. I'll check whether -### changes behaviour as expected and approve if it does

================
Comment at: clang/lib/Driver/ToolChains/AMDGPUOpenMP.cpp:297
+
+    std::string Arch = DAL->getLastArgValue(options::OPT_march_EQ).str();
+    if (Arch.empty()) {
----------------
This part is valuable as-is and probably independently testable, in that I think -### will have no march= before this change and will contain march=gfx*** after it. Though for that test to work reliably, I think it would have to run on a machine where amdgpu-arch succeeded, which is probably more bother than it's worth. Will check locally.

================
Comment at: clang/tools/clang-linker-wrapper/ClangLinkerWrapper.cpp:623
+  // Copy system library paths used by the host linker.
+  for (StringRef Arg : LinkerArgs)
+    if (Arg.startswith("-L"))
----------------
Does this mean to take the library paths the host used to link, and pass them unchanged for the device link as well? Doesn't seem a given that this is the right behaviour. Do we have a mechanism for passing arguments to the device linker? Might be ugly, e.g. `-Xopenmp-target=sm_70 -Xopenmp-target=-Wl,whatever`

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117246