[PATCH] D133726: [OpenMP][AMDGPU] Link bitcode ROCm device libraries per-TU

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl added inline comments.


================
Comment at: clang/lib/Driver/ToolChains/AMDGPU.cpp:717
 AMDGPUToolChain::getGPUArch(const llvm::opt::ArgList &DriverArgs) const {
-  return getProcessorFromTargetID(
-      getTriple(), DriverArgs.getLastArgValue(options::OPT_mcpu_EQ));
+  if (DriverArgs.hasArg(options::OPT_mcpu_EQ))
+    return getProcessorFromTargetID(
----------------
jhuber6 wrote:
> yaxunl wrote:
> > can we emit an error if both -march and -mcpu are specified and the values are different? This is a potential source of error.
> I'm not exactly sure about the semantics for this, maybe someone else can help. AFAIK `-mcpu` is sometimes treated as an alias of `-mtune` which is generally implied by `-march`. But having `-march` and `-mtune` state different things isn't technically disallowed. `-march` seems to imply that we generate code that can run on a certain architecture. Since these are in some respects equivalent it would probably be fine to just take the last one specified.
Some toolchains use -march to specify the target processor, some toolchains use -mcpu to specify the target processor. For AMDGPU toolchain, it uses -mcpu to specify the target processor in the beginning for OpenCL. Then we add HIP support and let HIPAMD toolchain adopt using -mcpu to specify the target processor. It is better to have a consistent way for specifying target processor for AMDGPU toolchains.

HIPAMD toolchain does this by https://github.com/llvm/llvm-project/blob/main/clang/lib/Driver/ToolChains/HIPAMD.cpp#L275 .

Probably AMDGPUOpenMP toolchain can make a similar change? Then we have a consistent way to specify target processor for AMDGPU toolchains.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133726