awarzynski added a comment.
Really nice to see some shared code being elevated out of Clang into LLVM,
thanks!
I've only reviewed on the Flang driver changes. I will let the OpenMP experts
to review the remaining bits. All in all looks good, I've only made some small
suggestions.
Thanks for working on this!
================
Comment at: flang/lib/Frontend/FrontendActions.cpp:93
+std::string CodeGenAction::getAllTargetFeatures() {
+ std::string allFeaturesStr;
----------------
This method could be simplified by following
https://llvm.org/docs/CodingStandards.html#use-early-exits-and-continue-to-simplify-code.
For example:
```
std::string CodeGenAction::getAllTargetFeatures() {
if (!triple.isAMDGPU()) {
allFeaturesStr = llvm::join(targetOpts.featuresAsWritten.begin(),
targetOpts.featuresAsWritten.end(), ",");
return allFeaturesStr;
}
// The logic for AMDGPU
// Perhaps add a dedicated hook: getExplicitAndImplicitAMDGPUTargetFeatures()
}
```
Btw, this method does different things depending on the triple. Perhaps rename
it as something more generic, e.g. `getTargetFeatures`? I think that the
current name, `getAllTargetFeatures`, is a bit misleading (i.e. what does "all"
mean?).
Also:
* make it `static`
* document
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D145579/new/
https://reviews.llvm.org/D145579
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
