[PATCH] D18369: [OpenCL] Upstreaming khronos OpenCL header file.

[Yaxun Liu via cfe-commits]

yaxunl added a comment.

In http://reviews.llvm.org/D18369#424347, @pxli168 wrote:

> In http://reviews.llvm.org/D18369#422367, @yaxunl wrote:
>
> > In http://reviews.llvm.org/D18369#421963, @pxli168 wrote:
> >
> > > If we want to save some space, could we use some macro to expand the gentype or some script to expand the gentype into each types when the clang is build?
> >
> >
> > We need to balance between space and readability. When I absorbed __attribute__((overloadable)) into __const_func to save space, it did not sacrifice readability. However using macro with gentype will cause the header file more difficult to read.
>
>
> But I don't think this kind of so long header is easy to read, it contains full pages of the same function with different types and it is hard to see if anyone is missing or find where these functions end. In spec builtin functions can be represented by
>
> > gentype ctz (gentype x)
>
> >  gentype max (gentype x, gentype y)
>
> >  gentype max (gentype x, sgentype y)
>
>
> If we could use some macro or script to generate the actual builtin functions, it seems more likely spec and clear to read, also will avoid missing or typo.


For simple cases, we could define something like this:

#define MATH_FUN(F) \
float F(float x); \
double F(double x); \
half F(half x);

MATH_FUN(sin)
MATH_FUN(cos)

It could be concise without sacrificing much clarity. However, for more complicated patterns, like

uchar8 __const_func convert_uchar8_rtn(int8);

It seems hard to balance between readability and brevity. Mostly it will ends up with multiple layers of macros calling each other, and the original function declarations obscured. This also opens Pandora's box of subjectivity of readability.

I think using macro and not using macro both have advantages and disadvantages. That's why I did not take efforts to change one representation to another.

It will take considerable efforts to define all builtin functions as macros, whereas this review already dragged too long.

Anastasia, what's your suggestion? Thanks.


http://reviews.llvm.org/D18369