[Libclc-dev] [PATCH 01/14] native_log2: Switch to generic native intrinsic inc file

Fri Nov 10 09:37:03 PST 2017

On Fri, Nov 10, 2017 at 10:52 AM, Jan Vesely <jan.vesely at rutgers.edu> wrote:
> On Fri, 2017-11-10 at 08:04 -0600, Aaron Watry via Libclc-dev wrote:
>> On Thu, 2017-11-09 at 16:05 -0500, Jan Vesely via Libclc-dev wrote:
>> > On Thu, 2017-11-09 at 21:57 +0100, Jeroen Ketema wrote:
>> > > > On 4 Nov 2017, at 00:07, Jan Vesely via Libclc-dev <libclc-dev at li
>> > > > sts.llvm.org> wrote:
>> > > >
>> > > > Signed-off-by: Jan Vesely <jan.vesely at rutgers.edu>
>> > > > ---
>> > > > generic/include/utils.h                             |  9
>> > > > +++++++++
>> > > > generic/lib/math/native_log2.cl                     |  8 ++------
>> > > > .../{native_log2.inc => native_unary_intrinsic.inc} | 21
>> > > > +++++++++++++++++++--
>> > > > 3 files changed, 30 insertions(+), 8 deletions(-)
>> > > > create mode 100644 generic/include/utils.h
>> > > > rename generic/lib/math/{native_log2.inc =>
>> > > > native_unary_intrinsic.inc} (65%)
>> > > >
>> > > > diff --git a/generic/include/utils.h b/generic/include/utils.h
>> > >
>> > > Does placing the file in generic/include mean that it will be
>> > > copied
>> > > upon installation? If so, why is that necessary?
>> >
>> > no, only the include files in generic/include/clc are installed.
>> > I've a cleanup to remove all but function declaration includes from
>> > include/clc on my todo list, but it will take some time.
>>
>> That was originally my question before I tested it...  That being said,
>> if it's not being installed, are you just keeping it in the include/
>> folder because it's used in sub-target overrides, or would it make
>> sense to move it to generic/lib/ as a sibling of clcmacro.h?
>
> I'd prefer if all private clc headers were in generic/include (maybe we
> should come up with a better name/location).
> Going forward I plan to remove all non-declaration files from
> generic/include/clc
> and provide either __clc_op and/or __clc_native_op (declared in
> generic/include/) for at least math functions, thus target
> implementation can be just wrappers and pick either one, depending on
> what we know about precision of native instructions.

I just wanted to make sure that this was a conscious decision.  I'm ok
with something along the lines of creating a
"generic/include/internal" or other named directory where we could
stick our private/non-installed headers.

--Aaron

> Jan
>
>>
>> --Aaron
>>
>> >
>> > >
>> > > > new file mode 100644
>> > > > index 0000000..23f4d34
>> > > > --- /dev/null
>> > > > +++ b/generic/include/utils.h
>> > > > @@ -0,0 +1,9 @@
>> > > > +
>> > > > +#ifndef __CLC_UTILS_H_
>> > > > +#define __CLC_UTILS_H_
>> > > > +
>> > > > +#define __CLC_CONCAT(x, y) x ## y
>> > > > +#define __CLC_STR(x) #x
>> > > > +#define __CLC_XSTR(x) __CLC_STR(x)
>> > > > +
>> > > > +#endif
>> > > > diff --git a/generic/lib/math/native_log2.cl
>> > > > b/generic/lib/math/native_log2.cl
>> > > > index 35ed18b..b610423 100644
>> > > > --- a/generic/lib/math/native_log2.cl
>> > > > +++ b/generic/lib/math/native_log2.cl
>> > > > @@ -22,11 +22,7 @@
>> > > >
>> > > > #include <clc/clc.h>
>> > > >
>> > > > -#define __CLC_FUNCTION __clc_native_log2
>> > > > -#define __CLC_INTRINSIC "llvm.log2"
>> > > > -#undef cl_khr_fp64
>> > > > -#include <clc/math/unary_intrin.inc>
>> > > > -
>> > > > -#define __CLC_BODY <native_log2.inc>
>> > > > +#define __CLC_NATIVE_INTRINSIC log2
>> > > > +#define __CLC_BODY <native_unary_intrinsic.inc>
>> > > > #define __FLOAT_ONLY
>> > > > #include <clc/math/gentype.inc>
>> > > > diff --git a/generic/lib/math/native_log2.inc
>> > > > b/generic/lib/math/native_unary_intrinsic.inc
>> > > > similarity index 65%
>> > > > rename from generic/lib/math/native_log2.inc
>> > > > rename to generic/lib/math/native_unary_intrinsic.inc
>> > > > index 0f6a509..048da8a 100644
>> > > > --- a/generic/lib/math/native_log2.inc
>> > > > +++ b/generic/lib/math/native_unary_intrinsic.inc
>> > > > @@ -20,6 +20,23 @@
>> > > >  * THE SOFTWARE.
>> > > >  */
>> > > >
>> > > > -_CLC_OVERLOAD _CLC_DEF __CLC_GENTYPE native_log2(__CLC_GENTYPE
>> > > > val) {
>> > > > -  return __clc_native_log2(val);
>> > > > +#include <utils.h>
>> > > > +
>> > > > +#ifdef __CLC_SCALAR
>> > > > +#define __CLC_FUNCTION __CLC_CONCAT(__clc_native,
>> > > > __CLC_NATIVE_INTRINSIC)
>> > > > +#define __CLC_INTRINSIC "llvm."
>> > > > __CLC_XSTR(__CLC_NATIVE_INTRINSIC)
>> > > > +
>> > > > +#undef cl_khr_fp64
>> > > > +#include <clc/math/unary_intrin.inc>
>> > > > +
>> > > > +#endif
>> > > > +
>> > > > +// For some reason this requires double redirection
>> > >
>> > > This comment is not very helpful, and is for the same reason you
>> > > need
>> > > __CLC_XSTR. I would just get rid of the comment.
>> >
>> > right, removed locally.
>> >
>> > thanks,
>> > Jan
>> >
>> > >
>> > > Jeroen
>> > >
>> > > > +#define __CLC_FUNCTION2(x) __CLC_CONCAT(native_, x)
>> > > > +#define __CLC_FUNCTION __CLC_FUNCTION2(__CLC_NATIVE_INTRINSIC)
>> > > > +
>> > > > +_CLC_OVERLOAD _CLC_DEF __CLC_GENTYPE
>> > > > __CLC_FUNCTION(__CLC_GENTYPE val) {
>> > > > +  return __CLC_CONCAT(__clc_native,
>> > > > __CLC_NATIVE_INTRINSIC)(val);
>> > > > }
>> > > > +#undef __CLC_FUNCTION2
>> > > > +#undef __CLC_FUNCTION
>> > > > --
>> > > > 2.13.6
>> > > >
>> > > > _______________________________________________
>> > > > Libclc-dev mailing list
>> > > > Libclc-dev at lists.llvm.org
>> > > > http://lists.llvm.org/cgi-bin/mailman/listinfo/libclc-dev
>> > >
>> > >
>> >
>> > _______________________________________________
>> > Libclc-dev mailing list
>> > Libclc-dev at lists.llvm.org
>> > http://lists.llvm.org/cgi-bin/mailman/listinfo/libclc-dev
>>
>> _______________________________________________
>> Libclc-dev mailing list
>> Libclc-dev at lists.llvm.org
>> http://lists.llvm.org/cgi-bin/mailman/listinfo/libclc-dev