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

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to