On 5/9/22 23:27, Joseph Myers wrote:
On Mon, 9 May 2022, Christophe Lyon via Gcc-patches wrote:
This patch adds support for trunc and extend operations between HF
mode (__fp16) and Decimal Floating Point formats (_Decimal32,
_Decimal64 and _Decimal128).
For simplicity we rely on the implicit conversions inserted by the
compiler between HF and SD/DF/TF modes. The existing bid*_to_binary*
and binary*_to_bid* functions are non-trivial and at this stage it is
not clear if there is a performance-critical use case involving __fp16
and _Decimal* formats.
Note that for conversion from DFP to HFmode, double rounding (going via
SFmode) probably produces incorrectly rounded results in some cases
(though we already have such incorrect results in the other direction for
DPD; see bug 97635).
Thanks for the pointer.
I was aware of such imprecision in the code I introduce, but not in
existing one. I don't think this is a blocker for this patch, is it?
The patch also adds two executable tests for AArch64, to make sure the
right functions are used, available (link phase) and functional.
Wouldn't it be better to have tests that apply for all targets supporting
both HFmode and BID DFP, which includes x86 / x86_64 (with SSE2 support)
as well, using _Float16 as the type name (and the float16 /
float16_runtime effective-target keywords to test for the relevant support
and dg-add-options float16)?
Indeed! What would be the appropriate directory for such tests?
+HFtype
+__bid_truncddhf (_Decimal64 x) {
+ HFtype res;
+ union decimal64 ux;
+
+ ux.d = x;
+ res = __bid64_to_binary32 (ux.i);
+ return (res);
+}
Doesn't this need appropriate LIBGCC2_HAS_HF_MODE || BID_HAS_HF_MODE
conditionals like some of the other functions you're adding?
Yes, you are right, I'll fix that.
+HFtype
+__bid_truncsdhf (_Decimal32 x) {
+ HFtype res;
+ union decimal32 ux;
+
+ ux.d = x;
+ res = __bid32_to_binary32 (ux.i);
+ return (res);
+}
Likewise.
OK
+HFtype
+__bid_trunctdhf (_Decimal128 x) {
+ HFtype res;
+ union decimal128 ux;
+
+ ux.d = x;
+ res = __bid128_to_binary32 (ux.i);
+ return (res);
+}
Likewise.
OK
+#if LIBGCC2_HAS_HF_MODE
+typedef float HFtype __attribute__ ((mode (HF)));
+#endif /* LIBGCC2_HAS_HF_MODE */
typedef float SFtype __attribute__ ((mode (SF)));
typedef float DFtype __attribute__ ((mode (DF)));
#if LIBGCC2_HAS_XF_MODE
@@ -98,6 +111,12 @@ typedef __attribute__ ((aligned(16))) struct
#endif
#endif
+#if BID_HAS_HF_MODE
+#ifndef HFtype
+#define HFtype __fp16
+#endif
That seems wrong; the __fp16 name is specific to Arm / AArch64. I'd
expect the typedef alone to be specific; in any case, the "mode" attribute
is the appropriate way to define this type.
I'll change that to __float16 (like __float128 we have a few lines below
for TFtype).
Thanks!
Christophe
