sameerds added inline comments.
================
Comment at: clang/lib/AST/Decl.cpp:3221
+ !hasAttr<CUDAHostAttr>()) ||
+ Context.getTargetInfo().getTriple().isAMDGCN()) &&
!(BuiltinID == Builtin::BIprintf || BuiltinID == Builtin::BImalloc))
----------------
sameerds wrote:
> This seems awkward to me. Why mix it up with only CUDA and HIP? The earlier
> factoring is better, where CUDA/HIP took care of their own business, and the
> catch-all case of AMDGCN was a separate clause by itself. It doesn't matter
> that the builtins being checked for AMDGCN on OpenMP are //currently//
> identical to CUDA/HIP. When this situation later changes (I am sure OpenMP
> will support more builtins), we will have to split it out again anyway.
This clause seems to assume that AMDGCN automatically means OpenMP whenever it
is not HIP. That does not sound right. Is there a Context.getLangOpts().OpenMP
flag? If not, why not? There also seems to be an Opts.OpenMPIsDevice ...
perhaps that could be used here to write a separate OpenMP clause?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D79754/new/
https://reviews.llvm.org/D79754
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
