[Libclc-dev] [PATCH 4/4] Implement step builtin

Tom Stellard tom at stellard.net
Mon Mar 2 07:05:33 PST 2015


On Thu, Feb 26, 2015 at 11:48:41AM -0600, Aaron Watry wrote:
> On Thu, Feb 26, 2015 at 10:59 AM, Tom Stellard <tom at stellard.net> wrote:
> 
> > On Thu, Feb 26, 2015 at 10:16:48AM -0600, Aaron Watry wrote:
> > > On Wed, Feb 25, 2015 at 3:20 PM, Tom Stellard <thomas.stellard at amd.com>
> > > wrote:
> > >
> > > > ---
> > > >  generic/include/clc/clc.h           |  1 +
> > > >  generic/include/clc/common/step.h   | 25 +++++++++++++++++
> > > >  generic/include/clc/common/step.inc | 28 +++++++++++++++++++
> > > >  generic/lib/SOURCES                 |  1 +
> > > >  generic/lib/clcmacro.h              | 23 ++++++++++++++++
> > > >  generic/lib/common/step.cl          | 54
> > > > +++++++++++++++++++++++++++++++++++++
> > > >  6 files changed, 132 insertions(+)
> > > >  create mode 100644 generic/include/clc/common/step.h
> > > >  create mode 100644 generic/include/clc/common/step.inc
> > > >  create mode 100644 generic/lib/common/step.cl
> > > >
> > > > diff --git a/generic/include/clc/clc.h b/generic/include/clc/clc.h
> > > > index e1f888b..cee1614 100644
> > > > --- a/generic/include/clc/clc.h
> > > > +++ b/generic/include/clc/clc.h
> > > > @@ -110,6 +110,7 @@
> > > >  #include <clc/common/radians.h>
> > > >  #include <clc/common/sign.h>
> > > >  #include <clc/common/smoothstep.h>
> > > > +#include <clc/common/step.h>
> > > >
> > > >  /* 6.11.5 Geometric Functions */
> > > >  #include <clc/geometric/cross.h>
> > > > diff --git a/generic/include/clc/common/step.h
> > > > b/generic/include/clc/common/step.h
> > > > new file mode 100644
> > > > index 0000000..9c0bee4
> > > > --- /dev/null
> > > > +++ b/generic/include/clc/common/step.h
> > > > @@ -0,0 +1,25 @@
> > > > +/*
> > > > + * Copyright (c) 2014,2015 Advanced Micro Devices, Inc.
> > > > + *
> > > > + * Permission is hereby granted, free of charge, to any person
> > obtaining
> > > > a copy
> > > > + * of this software and associated documentation files (the
> > "Software"),
> > > > to deal
> > > > + * in the Software without restriction, including without limitation
> > the
> > > > rights
> > > > + * to use, copy, modify, merge, publish, distribute, sublicense,
> > and/or
> > > > sell
> > > > + * copies of the Software, and to permit persons to whom the Software
> > is
> > > > + * furnished to do so, subject to the following conditions:
> > > > + *
> > > > + * The above copyright notice and this permission notice shall be
> > > > included in
> > > > + * all copies or substantial portions of the Software.
> > > > + *
> > > > + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
> > > > EXPRESS OR
> > > > + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
> > > > MERCHANTABILITY,
> > > > + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT
> > > > SHALL THE
> > > > + * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR
> > OTHER
> > > > + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE,
> > > > ARISING FROM,
> > > > + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
> > DEALINGS
> > > > IN
> > > > + * THE SOFTWARE.
> > > > + */
> > > > +
> > > > +#define __CLC_BODY <clc/common/step.inc>
> > > > +#include <clc/math/gentype.inc>
> > > > +#undef __CLC_BODY
> > > > diff --git a/generic/include/clc/common/step.inc
> > > > b/generic/include/clc/common/step.inc
> > > > new file mode 100644
> > > > index 0000000..f606b7c
> > > > --- /dev/null
> > > > +++ b/generic/include/clc/common/step.inc
> > > > @@ -0,0 +1,28 @@
> > > > +/*
> > > > + * Copyright (c) 2014,2015 Advanced Micro Devices, Inc.
> > > > + *
> > > > + * Permission is hereby granted, free of charge, to any person
> > obtaining
> > > > a copy
> > > > + * of this software and associated documentation files (the
> > "Software"),
> > > > to deal
> > > > + * in the Software without restriction, including without limitation
> > the
> > > > rights
> > > > + * to use, copy, modify, merge, publish, distribute, sublicense,
> > and/or
> > > > sell
> > > > + * copies of the Software, and to permit persons to whom the Software
> > is
> > > > + * furnished to do so, subject to the following conditions:
> > > > + *
> > > > + * The above copyright notice and this permission notice shall be
> > > > included in
> > > > + * all copies or substantial portions of the Software.
> > > > + *
> > > > + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
> > > > EXPRESS OR
> > > > + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
> > > > MERCHANTABILITY,
> > > > + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT
> > > > SHALL THE
> > > > + * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR
> > OTHER
> > > > + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE,
> > > > ARISING FROM,
> > > > + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
> > DEALINGS
> > > > IN
> > > > + * THE SOFTWARE.
> > > > + */
> > > > +
> > > > +_CLC_OVERLOAD _CLC_DECL __CLC_GENTYPE step(__CLC_GENTYPE edge,
> > > > __CLC_GENTYPE x);
> > > > +_CLC_OVERLOAD _CLC_DECL __CLC_GENTYPE step(float edge, __CLC_GENTYPE
> > x);
> > > > +
> > > > +#ifdef cl_khr_fp64
> > > > +_CLC_OVERLOAD _CLC_DECL __CLC_GENTYPE step(double edge, __CLC_GENTYPE
> > x);
> > > > +#endif
> > > > diff --git a/generic/lib/SOURCES b/generic/lib/SOURCES
> > > > index aaedada..a7b2fa2 100644
> > > > --- a/generic/lib/SOURCES
> > > > +++ b/generic/lib/SOURCES
> > > > @@ -31,6 +31,7 @@ common/degrees.cl
> > > >  common/radians.cl
> > > >  common/sign.cl
> > > >  common/smoothstep.cl
> > > > +common/step.cl
> > > >  geometric/cross.cl
> > > >  geometric/dot.cl
> > > >  geometric/length.cl
> > > > diff --git a/generic/lib/clcmacro.h b/generic/lib/clcmacro.h
> > > > index 39636c9..346adf2 100644
> > > > --- a/generic/lib/clcmacro.h
> > > > +++ b/generic/lib/clcmacro.h
> > > > @@ -41,6 +41,29 @@
> > > >      return (RET_TYPE##16)(FUNCTION(x.lo, y.lo), FUNCTION(x.hi,
> > y.hi)); \
> > > >    }
> > > >
> > > > +#define _CLC_V_S_V_VECTORIZE(DECLSPEC, RET_TYPE, FUNCTION, ARG1_TYPE,
> > > > ARG2_TYPE) \
> > > > +  DECLSPEC RET_TYPE##2 FUNCTION(ARG1_TYPE x, ARG2_TYPE##2 y) { \
> > > > +    return (RET_TYPE##2)(FUNCTION(x, y.lo), FUNCTION(x, y.hi)); \
> > > > +  } \
> > > > +\
> > > > +  DECLSPEC RET_TYPE##3 FUNCTION(ARG1_TYPE x, ARG2_TYPE##3 y) { \
> > > > +    return (RET_TYPE##3)(FUNCTION(x, y.x), FUNCTION(x, y.y), \
> > > > +                         FUNCTION(x, y.z)); \
> > > > +  } \
> > > > +\
> > > > +  DECLSPEC RET_TYPE##4 FUNCTION(ARG1_TYPE x, ARG2_TYPE##4 y) { \
> > > > +    return (RET_TYPE##4)(FUNCTION(x, y.lo), FUNCTION(x, y.hi)); \
> > > > +  } \
> > > > +\
> > > > +  DECLSPEC RET_TYPE##8 FUNCTION(ARG1_TYPE x, ARG2_TYPE##8 y) { \
> > > > +    return (RET_TYPE##8)(FUNCTION(x, y.lo), FUNCTION(x, y.hi)); \
> > > > +  } \
> > > > +\
> > > > +  DECLSPEC RET_TYPE##16 FUNCTION(ARG1_TYPE x, ARG2_TYPE##16 y) { \
> > > > +    return (RET_TYPE##16)(FUNCTION(x, y.lo), FUNCTION(x, y.hi)); \
> > > > +  } \
> > > > +\
> > > > +
> > > >  #define _CLC_TERNARY_VECTORIZE(DECLSPEC, RET_TYPE, FUNCTION,
> > ARG1_TYPE,
> > > > ARG2_TYPE, ARG3_TYPE) \
> > > >    DECLSPEC RET_TYPE##2 FUNCTION(ARG1_TYPE##2 x, ARG2_TYPE##2 y,
> > > > ARG3_TYPE##2 z) { \
> > > >      return (RET_TYPE##2)(FUNCTION(x.x, y.x, z.x), FUNCTION(x.y, y.y,
> > > > z.y)); \
> > > > diff --git a/generic/lib/common/step.cl b/generic/lib/common/step.cl
> > > > new file mode 100644
> > > > index 0000000..4b022f1
> > > > --- /dev/null
> > > > +++ b/generic/lib/common/step.cl
> > > > @@ -0,0 +1,54 @@
> > > > +/*
> > > > + * Copyright (c) 2014,2015 Advanced Micro Devices, Inc.
> > > > + *
> > > > + * Permission is hereby granted, free of charge, to any person
> > obtaining
> > > > a copy
> > > > + * of this software and associated documentation files (the
> > "Software"),
> > > > to deal
> > > > + * in the Software without restriction, including without limitation
> > the
> > > > rights
> > > > + * to use, copy, modify, merge, publish, distribute, sublicense,
> > and/or
> > > > sell
> > > > + * copies of the Software, and to permit persons to whom the Software
> > is
> > > > + * furnished to do so, subject to the following conditions:
> > > > + *
> > > > + * The above copyright notice and this permission notice shall be
> > > > included in
> > > > + * all copies or substantial portions of the Software.
> > > > + *
> > > > + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
> > > > EXPRESS OR
> > > > + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
> > > > MERCHANTABILITY,
> > > > + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT
> > > > SHALL THE
> > > > + * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR
> > OTHER
> > > > + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE,
> > > > ARISING FROM,
> > > > + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
> > DEALINGS
> > > > IN
> > > > + * THE SOFTWARE.
> > > > + */
> > > > +
> > > > +#include <clc/clc.h>
> > > > +
> > > > +#include "../clcmacro.h"
> > > > +
> > > > +_CLC_OVERLOAD _CLC_DEF float step(float edge, float x) {
> > > > +  return x < edge ? 0.0f : 1.0f;
> > > > +}
> > > > +
> > > > +_CLC_BINARY_VECTORIZE(_CLC_OVERLOAD _CLC_DEF, float, step, float,
> > float);
> > > > +
> > > > +_CLC_V_S_V_VECTORIZE(_CLC_OVERLOAD _CLC_DEF, float, step, float,
> > float);
> > > > +
> > > > +#ifdef cl_khr_fp64
> > > > +#pragma OPENCL EXTENSION cl_khr_fp64 : enable
> > > > +
> > > > +#define STEP_DEF(edge_type, x_type) \
> > > > +  _CLC_OVERLOAD _CLC_DEF x_type step(edge_type edge, x_type x) { \
> > > > +    return x < edge ? 0.0 : 1.0; \
> > > > + }
> > > > +
> > > > +STEP_DEF(double, double);
> > > > +
> > > > +_CLC_BINARY_VECTORIZE(_CLC_OVERLOAD _CLC_DEF, double, step, double,
> > > > double);
> > > > +_CLC_V_S_V_VECTORIZE(_CLC_OVERLOAD _CLC_DEF, double, step, double,
> > > > double);
> > > > +
> > > >
> > > +STEP_DEF(float, double);
> > > > +STEP_DEF(double, float);
> > > > +
> > > > +_CLC_V_S_V_VECTORIZE(_CLC_OVERLOAD _CLC_DEF, double, step, float,
> > double);
> > > > +_CLC_V_S_V_VECTORIZE(_CLC_OVERLOAD _CLC_DEF, float, step, double,
> > float);
> > > > +
> > > >
> > > This patch looks good up until here.  Did you mean to define:
> > > step(float edge, double x) and step(double edge, float x)
> > >
> > > I don't see those variants in the spec, but all I've checked is the 1.x
> > > quick reference cards and the CLC 2.0 spec section 6.13.4.
> > >
> >
> > Both the 1.0 and 1.1 man pages have:
> >
> > gentype step (float edge, gentype x)
> > gentype step (double edge, gentype x)
> >
> > My interpretation is that since gentype can be either float or double,
> > the mixed variants are required.
> >
> > -Tom
> >
> 
> I got most of the way through writing something about how because step is
> defined separately in CL1.1 sections 6.11.4 and 9.3.3 and therefore they
> weren't actually overlapping, but then I reread the section introduction,
> which states:
> 
> "The built-in common functions defined in table 6.12 (also listed below)
> are extended to include
> appropriate versions of functions that take double, and double{2|3|4|8|16}
> as
> arguments and return values. gentype now also includes double, double2,
> double3,
> double4, double8 and double16."
> 
> The key words being "gentype NOW ALSO INCLUDES", which means that the float
> values are actually included, and the additional variants should be
> allowed.  My bad.
> 
> I'm fine with this as it stands.
> 
> Do we need to re-check the clamp, min, max, mix, and smoothstep functions
> for similar gotchas? Or are those correct already?  Most of the test suites
> that I have access to are pretty lacking in double tests and the hardware
> that I'm sitting in front ATM of doesn't even support doubles.
> 

I can take a look at this.

For this current series if I add a note about conformance test passing are these
OK to commit?

Thanks,
Tom

> --Aaron
> 
> 
> > > With the mixed float/double variants removed:
> > > Reviewed-by: Aaron Watry <awatry at gmail.com>
> > >
> > >
> > >
> > > > +#endif
> > > > --
> > > > 2.0.4
> > > >
> > > >
> > > > _______________________________________________
> > > > 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
> >
> >




More information about the Libclc-dev mailing list