[PATCH] D107769: [OpenCL] Make generic addrspace optional for -fdeclare-opencl-builtins

[Sven van Haastregt via Phabricator via cfe-commits]

svenvh added a comment.

In D107769#3265665 <https://reviews.llvm.org/D107769#3265665>, @Anastasia wrote:

> The way I understand the spec for OpenCL C 2.0 is that whenever the address space of the pointer is not listed it means generic has to be used, here is one example:
> https://www.khronos.org/registry/OpenCL/specs/2.2/html/OpenCL_C.html#vector-data-load-and-store-functions
>
>   gentypen vloadn(size_t offset, const gentype *p)
>   gentypen vloadn(size_t offset, const __constant gentype *p)
>
> that has no address space (i.e. `generic`) and `constant` explicitly. So I think if address spaces are not listed explicitly they are not supposed to be available.

The unified specification (which "specifies all versions of OpenCL C") seems to be making all overloads available as I understand; it is perhaps subtly different from the previous specification?

https://www.khronos.org/registry/OpenCL/specs/3.0-unified/html/OpenCL_C.html#vector-data-load-and-store-functions

  gentypen vloadn(size_t offset, const __global gentype *p)
  gentypen vloadn(size_t offset, const __local gentype *p)
  gentypen vloadn(size_t offset, const __constant gentype *p)
  gentypen vloadn(size_t offset, const __private gentype *p)
  
  For OpenCL C 2.0, or OpenCL C 3.0 or newer with the __opencl_c_generic_address_space feature:
  
  gentypen vloadn(size_t offset, const gentype *p)

Since the `__constant` overload should always be available, I think a reader can assume that the overloads directly above and below `__constant` are also always available?  So that the generic overload is an optional addition to the list of overloads.  If not, I'd expect the spec to specify a condition before listing the specific overloads.

> One implication of adding all address space overloads is that it makes library size larger, but my feeling is that we don't have that many functions with pointers to significantly impace the library size?

This patch should be touching all of them.  Not that many indeed, but it might still have a non-negligible impact on OpenCL libraries due to the combination of #vector-sizes * #types * #addrspaces.


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

https://reviews.llvm.org/D107769