jhuber6 added inline comments.
================
Comment at: clang/lib/Driver/ToolChains/AMDGPUOpenMP.cpp:297
+
+ std::string Arch = DAL->getLastArgValue(options::OPT_march_EQ).str();
+ if (Arch.empty()) {
----------------
JonChesterfield wrote:
> 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.
Yes, I believe with this we could remove the other calls to
`checkSystemForAMDGPU` and just reference the `-march` in the driver arguments.
Not sure about testing because it requires the binary of course.
================
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"))
----------------
JonChesterfield wrote:
> 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`
That's a good idea for general control over the arguments to the linker
wrapper. I don't think the `-L` arguments are strictly necessary here. Could
probably move to your approach if needed, the `-L` arguments were already
present in the `clang-nvlink-wrapper` so I just replicated that without really
caring if it was necessary.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D117246/new/
https://reviews.llvm.org/D117246
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
