[Libclc-dev] [PATCH 4/4] Implement step builtin
Aaron Watry
awatry at gmail.com
Mon Mar 2 07:17:22 PST 2015
On Mon, Mar 2, 2015 at 9:05 AM, Tom Stellard <tom at stellard.net> wrote:
> 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?
>
>
Yeah, go ahead. I'm fine with the implementations as they currently stand.
Any of the other builtins that might be missing prototypes for specific
argument types can be handled in a follow-up if needed.
--Aaron
> 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
> > >
> > >
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/libclc-dev/attachments/20150302/2737a235/attachment.html>
More information about the Libclc-dev
mailing list