jhuber6 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(
----------------
yaxunl wrote:
> 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.
The rest of the OpenMP toolchain uses `-march` so I don't see a compelling
reason to change it. If this is a stopper I'll just remove the changes to this
function and check `-march` directly in the new `getROCmDeviceLibs` function
for the OpenMP toolchain.
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
