[Libclc-dev] Add initial support for half precision builtins (take 3)
Jeroen Ketema via Libclc-dev
libclc-dev at lists.llvm.org
Tue May 15 13:46:43 PDT 2018
> On 15 May 2018, at 05:09, Jan Vesely via Libclc-dev <libclc-dev at lists.llvm.org> wrote:
> 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.
#if __CLC_FPSIZE == 64
#define __CLC_NATN __CLC_XCONCAT(ulong, __CLC_VECSIZE)
+#elif __CLC_FPSIZE == 32
#define __CLC_NATN __CLC_XCONCAT(uint, __CLC_VECSIZE)
+#define __CLC_NATN __CLC_XCONCAT(ushort, __CLC_VECSIZE)
Wouldn’t it make more sense to also use “#elif __CLC_FPSIZE == 16”, and maybe #error otherwise? Similar in a few other places.
+#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?
> This should be enough to make clpeak happy (BZ: 96897)
> 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
More information about the Libclc-dev