[PATCH] D124721: [OpenMP] Allow compiling multiple target architectures with OpenMP

[Artem Belevich via Phabricator via cfe-commits]

tra accepted this revision.
tra added a comment.
This revision is now accepted and ready to land.

LGTM in general.



================
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);
----------------
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);
}
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124721