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

[Saiyedul Islam via Phabricator via cfe-commits]

saiislam marked 19 inline comments as done.
saiislam added inline comments.


================
Comment at: clang/lib/AST/Decl.cpp:3227
+      !(BuiltinID == Builtin::BIprintf || BuiltinID == Builtin::BImalloc))
+    return 0;
+
----------------
sameerds wrote:
> sameerds wrote:
> > This needs a test.
> This still needs a test. One that shows that specific builtins are allowed and others are not.
Added test case to show difference in treatment of printf() and other predefined library functions, on the device.


================
Comment at: clang/lib/AST/Decl.cpp:3226
+  // the only special cases that are supported by device-side runtime.
+  if (Context.getTargetInfo().getTriple().isAMDGCN() &&
+      Context.getLangOpts().OpenMPIsDevice &&
----------------
sameerds wrote:
> Why is the check for AMDGCN required here? Doesn't the language define the set of supported builtins in a language-independent way? At least that seems to be true for OpenCL and CUDA above.
AMDGCN check is required because this block deals with the openmp target when it is amdgcn. A future or existing target device might decide to deal with library calls in its own way.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79754