[PATCH] D79754: [OpenMP][AMDGCN] Support OpenMP offloading for AMDGCN architecture - Part 1

[Sameer Sahasrabuddhe via Phabricator via cfe-commits]

sameerds added inline comments.


================
Comment at: clang/lib/AST/Decl.cpp:3224
 
+  if (Context.getTargetInfo().getTriple().isAMDGCN() &&
+      Context.BuiltinInfo.isPredefinedLibFunction(BuiltinID) &&
----------------
arsenm wrote:
> This is also identical to the cuda handling above, can you merge them
It's not identical, because CUDA is filtering out host code and its check is target independent.

But then, Saiyed, it seems that the new patch disallows library builtins on all languages that reach this point, both on host and device code. Is that the intention? Does this point in the flow preclude any side-effects outside of "OpenMP on AMDGCN"?


================
Comment at: llvm/include/llvm/ADT/Triple.h:696
+  /// Tests whether the target is AMDGCN
+  bool isAMDGCN() const { return getArch() == Triple::amdgcn; }
+
----------------
Why not just isAMDGPU()? I myself don't have an opinion either way, but still curious.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79754