[PATCH] D118605: [OpenCL] Add support of language builtins for OpenCL C 3.0

Tue Feb 1 05:52:08 PST 2022

azabaznov added inline comments.

================
Comment at: clang/include/clang/Basic/Builtins.def:88
 //       '__builtin_' prefix. It will be implemented in compiler-rt or libgcc.
+//  G -> this function uses generic address space (OpenCL).
+//  P -> this function uses pipes (OpenCL).
----------------
Anastasia wrote:
> Anastasia wrote:
> > It might be better to avoid adding such limited language-specific functionality into generic representation of Builtins. Do you think could we could just introduce specific language modes, say:
> > 
> > `OCLC_PIPES`
> > `OCLC_DSE`
> > `OCLC_GAS`
> > 
> > and then check against those in `builtinIsSupported`?
> Btw another approach could be to do something similar to `TARGET_BUILTIN` i.e. list features in the last parameter as strings. We could add a separate macro for such builtins and just reuse target Builtins flow. This might be a bit more scalable in case we would need to add more of such builtins later on?
> 
> It might be better to avoid adding such limited language-specific functionality into generic representation of Builtins.

Nice idea! Though I think LanguageID is not designed to be used this way, it's used only to check against specific language version. So it seems pretty invasive. Also, function attributes seem more natural to me to specify the type of the function. I don't know for sure which way is better...

> Btw another approach could be to do something similar to TARGET_BUILTIN i.e. list features in the last parameter as strings.

I'd prefer to not use TARGET_BUILTIN as it operates on target feature, but OpenCL feature is some other concept in clang...

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118605