[Libclc-dev] [PATCH 3/7] relational/signbit: Refactor to use relational macros

Wed Jul 16 11:16:23 PDT 2014

On Wed, Jul 16, 2014 at 01:04:59PM -0500, Aaron Watry wrote:
> On Wed, Jul 16, 2014 at 12:45 PM, Matt Arsenault <arsenm2 at gmail.com> wrote:
> >
> > On Jul 16, 2014, at 10:35 AM, Tom Stellard <tom at stellard.net> wrote:
> >
> > On Wed, Jul 16, 2014 at 10:00:48AM -0700, Matt Arsenault wrote:
> >
> >
> > On Jul 16, 2014, at 8:05 AM, Tom Stellard <tom at stellard.net> wrote:
> >
> > On Tue, Jul 15, 2014 at 05:24:47PM -0500, Aaron Watry wrote:
> >
> > Signed-off-by: Aaron Watry <awatry at gmail.com>
> > ---
> > generic/include/clc/relational/signbit.h | 21 +++-------
> > generic/lib/relational/signbit.cl        | 72
> > +-------------------------------
> > 2 files changed, 8 insertions(+), 85 deletions(-)
> >
> > diff --git a/generic/include/clc/relational/signbit.h
> > b/generic/include/clc/relational/signbit.h
> > index 774d6e0..41e5284 100644
> > --- a/generic/include/clc/relational/signbit.h
> > +++ b/generic/include/clc/relational/signbit.h
> > @@ -1,18 +1,9 @@
> > +#undef signbit
> >
> >
> > What happens if someone compiles a kernel with:
> > -Dsignbit=__builtin_amdgpu_signbit
> >
> > Won't the undef here break this kernel?  Are these kinds of defines
> > even allowed?
> >
> > -Tom
> >
> >
> > I also think defining most library function as macros could have unpleasant
> > consequences on warnings. Is the final clc header preprocessed at install
> > time or anything like that? I also noticed that none of the function are
> > currently marked with __attribute__((const)), but nearly every function
> > should have it.
> >
> >
> > All of the headers are installed.  Can we fix all of our macro issues by
> > just
> > installing a single preprocessed clc.h.  Are there any downsides to doing
> > this?
> 
> If we do this, do we have to do anything special for cases where
> half/double aren't supported? We probably won't know that until
> runtime, and it'd be nice to know at compile time that you're
> attempting to use something that won't work rather than link/execution
> time.  Not sure, just asking.
> 

This is a problem now since we unconditionally add -Dcl_khr_fp64 to the
list of defines in libclc.

I was planning to add cl_khr_fp64 to the list of target defines in clang, so
that doubles (and also half types) would be enabled automatically on targets
that support it.

> --Aaron
> 
> >
> >
> > I’m not sure. The installed header should really go one step further and be
> > a precompiled header to help with compile time.
> >
> >
> > What does __attribute__((const)) do?
> >
> >
> > It’s the C equivalent of readnone (and pure is readonly). It’s probably
> > inferable in most cases assuming the intrinsics are correct, but it might
> > help with compile time to have it there from the beginning.
> >
> >
> >
> > -Tom
> >
> >
> >
> > -#define _CLC_SIGNBIT_DECL(TYPE, RETTYPE) \
> > -  _CLC_OVERLOAD _CLC_DECL RETTYPE signbit(TYPE x);
> > +#define __CLC_FUNCTION signbit
> > +#define __CLC_BODY <clc/relational/unary_decl.inc>
> >
> > -#define _CLC_VECTOR_SIGNBIT_DECL(TYPE, RETTYPE) \
> > -  _CLC_SIGNBIT_DECL(TYPE##2, RETTYPE##2)  \
> > -  _CLC_SIGNBIT_DECL(TYPE##3, RETTYPE##3)  \
> > -  _CLC_SIGNBIT_DECL(TYPE##4, RETTYPE##4)  \
> > -  _CLC_SIGNBIT_DECL(TYPE##8, RETTYPE##8)  \
> > -  _CLC_SIGNBIT_DECL(TYPE##16, RETTYPE##16)
> > +#include <clc/relational/floatn.inc>
> >
> > -_CLC_SIGNBIT_DECL(float, int)
> > -_CLC_VECTOR_SIGNBIT_DECL(float, int)
> > -
> > -#ifdef cl_khr_fp64
> > -_CLC_SIGNBIT_DECL(double, int)
> > -_CLC_VECTOR_SIGNBIT_DECL(double, long)
> > -#endif
> > \ No newline at end of file
> > +#undef __CLC_BODY
> > +#undef __CLC_FUNCTION
> > diff --git a/generic/lib/relational/signbit.cl
> > b/generic/lib/relational/signbit.cl
> > index f429960..ab37d2f 100644
> > --- a/generic/lib/relational/signbit.cl
> > +++ b/generic/lib/relational/signbit.cl
> > @@ -1,61 +1,5 @@
> > #include <clc/clc.h>
> > -
> > -#define _CLC_DEFINE_RELATIONAL_UNARY_SCALAR(RET_TYPE, FUNCTION,
> > BUILTIN_NAME, ARG_TYPE) \
> > -_CLC_DEF _CLC_OVERLOAD RET_TYPE FUNCTION(ARG_TYPE x){ \
> > - return BUILTIN_NAME(x); \
> > -} \
> > -
> > -#define _CLC_DEFINE_RELATIONAL_UNARY_VEC(RET_TYPE, FUNCTION, ARG_TYPE) \
> > -_CLC_DEF _CLC_OVERLOAD RET_TYPE FUNCTION(ARG_TYPE x) { \
> > -  return (RET_TYPE)( (RET_TYPE){FUNCTION(x.lo), FUNCTION(x.hi)} !=
> > (RET_TYPE)0); \
> > -} \
> > -
> > -#define _CLC_DEFINE_RELATIONAL_UNARY_VEC2(RET_TYPE, FUNCTION, ARG_TYPE) \
> > -_CLC_DEF _CLC_OVERLOAD RET_TYPE FUNCTION(ARG_TYPE x) { \
> > -  return (RET_TYPE)( (RET_TYPE){FUNCTION(x.lo), FUNCTION(x.hi)} !=
> > (RET_TYPE)0); \
> > -} \
> > -
> > -#define _CLC_DEFINE_RELATIONAL_UNARY_VEC3(RET_TYPE, FUNCTION, ARG_TYPE) \
> > -_CLC_DEF _CLC_OVERLOAD RET_TYPE FUNCTION(ARG_TYPE x) { \
> > -  return (RET_TYPE)( (RET_TYPE){FUNCTION(x.s0), FUNCTION(x.s1),
> > FUNCTION(x.s2)} != (RET_TYPE)0); \
> > -} \
> > -
> > -#define _CLC_DEFINE_RELATIONAL_UNARY_VEC4(RET_TYPE, FUNCTION, ARG_TYPE) \
> > -_CLC_DEF _CLC_OVERLOAD RET_TYPE FUNCTION(ARG_TYPE x) { \
> > -  return (RET_TYPE)( \
> > - (RET_TYPE){ \
> > - FUNCTION(x.s0), FUNCTION(x.s1), FUNCTION(x.s2), FUNCTION(x.s3) \
> > - } != (RET_TYPE)0); \
> > -} \
> > -
> > -#define _CLC_DEFINE_RELATIONAL_UNARY_VEC8(RET_TYPE, FUNCTION, ARG_TYPE) \
> > -_CLC_DEF _CLC_OVERLOAD RET_TYPE FUNCTION(ARG_TYPE x) { \
> > -  return (RET_TYPE)( \
> > - (RET_TYPE){ \
> > - FUNCTION(x.s0), FUNCTION(x.s1), FUNCTION(x.s2), FUNCTION(x.s3), \
> > - FUNCTION(x.s4), FUNCTION(x.s5), FUNCTION(x.s6), FUNCTION(x.s7) \
> > - } != (RET_TYPE)0); \
> > -} \
> > -
> > -#define _CLC_DEFINE_RELATIONAL_UNARY_VEC16(RET_TYPE, FUNCTION, ARG_TYPE) \
> > -_CLC_DEF _CLC_OVERLOAD RET_TYPE FUNCTION(ARG_TYPE x) { \
> > -  return (RET_TYPE)( \
> > - (RET_TYPE){ \
> > - FUNCTION(x.s0), FUNCTION(x.s1), FUNCTION(x.s2), FUNCTION(x.s3), \
> > - FUNCTION(x.s4), FUNCTION(x.s5), FUNCTION(x.s6), FUNCTION(x.s7), \
> > - FUNCTION(x.s8), FUNCTION(x.s9), FUNCTION(x.sa), FUNCTION(x.sb), \
> > - FUNCTION(x.sc), FUNCTION(x.sd), FUNCTION(x.se), FUNCTION(x.sf) \
> > - } != (RET_TYPE)0); \
> > -} \
> > -
> > -
> > -#define _CLC_DEFINE_RELATIONAL_UNARY(RET_TYPE, FUNCTION, BUILTIN_FUNCTION,
> > ARG_TYPE) \
> > -_CLC_DEFINE_RELATIONAL_UNARY_SCALAR(RET_TYPE, FUNCTION, BUILTIN_FUNCTION,
> > ARG_TYPE) \
> > -_CLC_DEFINE_RELATIONAL_UNARY_VEC2(RET_TYPE##2, FUNCTION, ARG_TYPE##2) \
> > -_CLC_DEFINE_RELATIONAL_UNARY_VEC3(RET_TYPE##3, FUNCTION, ARG_TYPE##3) \
> > -_CLC_DEFINE_RELATIONAL_UNARY_VEC4(RET_TYPE##4, FUNCTION, ARG_TYPE##4) \
> > -_CLC_DEFINE_RELATIONAL_UNARY_VEC8(RET_TYPE##8, FUNCTION, ARG_TYPE##8) \
> > -_CLC_DEFINE_RELATIONAL_UNARY_VEC16(RET_TYPE##16, FUNCTION, ARG_TYPE##16) \
> > +#include "relational.h"
> >
> > _CLC_DEFINE_RELATIONAL_UNARY(int, signbit, __builtin_signbitf, float)
> >
> > @@ -70,18 +14,6 @@ _CLC_DEF _CLC_OVERLOAD int signbit(double x){
> > return __builtin_signbit(x);
> > }
> >
> > -_CLC_DEFINE_RELATIONAL_UNARY_VEC2(long2, signbit, double2)
> > -_CLC_DEFINE_RELATIONAL_UNARY_VEC3(long3, signbit, double3)
> > -_CLC_DEFINE_RELATIONAL_UNARY_VEC4(long4, signbit, double4)
> > -_CLC_DEFINE_RELATIONAL_UNARY_VEC8(long8, signbit, double8)
> > -_CLC_DEFINE_RELATIONAL_UNARY_VEC16(long16, signbit, double16)
> > +_CLC_DEFINE_RELATIONAL_UNARY_VEC_ALL(long, signbit, double)
> >
> > #endif
> > -
> > -#undef _CLC_DEFINE_RELATIONAL_UNARY
> > -#undef _CLC_DEFINE_RELATIONAL_UNARY_SCALAR
> > -#undef _CLC_DEFINE_RELATIONAL_UNARY_VEC2
> > -#undef _CLC_DEFINE_RELATIONAL_UNARY_VEC3
> > -#undef _CLC_DEFINE_RELATIONAL_UNARY_VEC4
> > -#undef _CLC_DEFINE_RELATIONAL_UNARY_VEC8
> > -#undef _CLC_DEFINE_RELATIONAL_UNARY_VEC16
> > \ No newline at end of file
> > --
> > 1.9.1
> >
> >
> > _______________________________________________
> > Libclc-dev mailing list
> > Libclc-dev at pcc.me.uk
> > http://www.pcc.me.uk/cgi-bin/mailman/listinfo/libclc-dev
> >
> >
> > _______________________________________________
> > Libclc-dev mailing list
> > Libclc-dev at pcc.me.uk
> > http://www.pcc.me.uk/cgi-bin/mailman/listinfo/libclc-dev
> >
> >