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

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to