Anastasia added a comment.

In http://reviews.llvm.org/D18369#424617, @yaxunl wrote:

> 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.


Let's don't exaggerate with macros for this. Using them has negative effect of 
diagnostics being reported about a macro and not the original function, which 
won't conform the Spec any longer.

See my earlier comment to line 6582:

  error: too many arguments provided to function-like macro invocation


================
Comment at: lib/Headers/opencl-c.h:14057
@@ +14056,3 @@
+event_t __attribute__((overloadable)) async_work_group_copy(__local float2 
*dst, const __global float2 *src, size_t num_elements, event_t event);
+event_t __attribute__((overloadable)) async_work_group_copy(__local char3 
*dst, const __global char3 *src, size_t num_elements, event_t event);
+event_t __attribute__((overloadable)) async_work_group_copy(__local uchar3 
*dst, const __global uchar3 *src, size_t num_elements, event_t event);
----------------
yaxunl wrote:
> If this representation is not generic enough. Any suggestion for an 
> alternative? Thanks.
I don't think Spec imposes any specific implementation of this macro.

I am thinking we might better leave it out to allow adding in a way suitable 
for other implementations.


http://reviews.llvm.org/D18369



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

Reply via email to