domada marked an inline comment as done.
domada added inline comments.
================
Comment at: flang/lib/Frontend/FrontendActions.cpp:93
+std::string CodeGenAction::getAllTargetFeatures() {
+ std::string allFeaturesStr;
----------------
awarzynski wrote:
> 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
Hi,
thanks for the review. I applied your remarks. I also moved OpenMP related
changes to the child review.
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
