jhuber6 added inline comments.
================
Comment at: clang/lib/Driver/ToolChains/AMDGPUOpenMP.cpp:313
+ if (BoundArch.empty())
+ checkSystemForAMDGPU(Args, *this, Arch);
DAL->AddJoinedArg(nullptr, Opts.getOption(options::OPT_march_EQ), Arch);
----------------
tra wrote:
> I'd change `checkSystemForAMDGPU` to return the Arch or empty string.
>
> I'd also simplify the code to something like this:
> ```
> std::string Arch = DAL->getLastArgValue(options::OPT_march_EQ).str();
> if (Arch.empty()) {
> Arch = !BoundArch.empty() ? BoundArch : checkSystemForAMDGPU(Args, *this);
> DAL->AddJoinedArg(nullptr, Opts.getOption(options::OPT_march_EQ), Arch);
> }
> ```
Having `checkSystemForAMDGPU` cause errors when trying to test this was a pain.
It'll take a bit more plumbing to address that so I think I'll leave this as-is
for this patch and address it in a follow-up.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D124721/new/
https://reviews.llvm.org/D124721
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
