[Libclc-dev] Add initial support for half precision builtins (take 3)

[Jeroen Ketema via Libclc-dev]

Hi Jan,

> On 15 May 2018, at 05:09, Jan Vesely via Libclc-dev <libclc-dev at lists.llvm.org> wrote:
> 
> Hi,
> 
> I've tried to post this patch but it's too big for the ML (and I don't
> know who to bug to get it released).
> 
> It adds basic fp16 builtins. The rest will probably need a fp32 wrapper
> (especially the sw implementations of math functions), but that needs
> fixed conversion routines.

Is that going to fly? Wouldn’t you run into double rounding issues?

In addition to Aaron’s comments:

+#define HALF_RADIX 	2
+#define HALF_MAX 	0x1.ffcp15h
+#define HALF_MIN 	0x1.0p-14h
+#define HALF_EPSILON 	0x1.0p-10h

There seems to be both a space and a tab between the macro name and its definition. I also see a few places where indentation is done with a tab, e.g., generic/lib/relational/isunordered.cl.

From nan.inc:

 #if __CLC_FPSIZE == 64
 #define __CLC_NATN __CLC_XCONCAT(ulong, __CLC_VECSIZE)
-#else
+#elif __CLC_FPSIZE == 32
 #define __CLC_NATN __CLC_XCONCAT(uint, __CLC_VECSIZE)
+#else
+#define __CLC_NATN __CLC_XCONCAT(ushort, __CLC_VECSIZE)
 #endif

Wouldn’t it make more sense to also use “#elif __CLC_FPSIZE == 16”, and maybe #error otherwise? Similar in a few other places.

In generic/lib/math/acos.inc:

+#else
+#define __CLC_CONST(x) xh

I think there should be a ## between the x and h.

In generic/include/math/clc_ldexp.h, the guarded text in indented, while indentation is not used anywhere else.

I see some cases where the "+#if __CLC_FPSIZE > 16” guard does not have a TODO attached to it, is that on purpose?

In generic/lib/math/modf.inc casts 0.0f, while generic/lib/math/fract.inc introduces a ZERO macro. Maybe this should be made more uniform (doesn’t have to be as part of this patch).

Near the end of the patch the int/vector of short difference is explained in a comment "returns an int, but the vector versions return short”. Maybe add a similar comment to generic/include/clc/relational/floatn.inc?

Jeroen

> 
> This should be enough to make clpeak happy (BZ: 96897)
> 
> Jan
> -- 
> Jan Vesely <jan.vesely at rutgers.edu><0001-Add-initial-support-for-half-precision-builtins.patch.gz>_______________________________________________
> Libclc-dev mailing list
> Libclc-dev at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/libclc-dev