Hi Richard,
on 2024/7/8 19:14, Richard Sandiford wrote:
> "Kewen.Lin" <li...@linux.ibm.com> writes:[snip...]
>>>
>>> This part looks good to me FWIW, but what's the correct behaviour of:
>>>
>>> if (GET_MODE_PRECISION (from_mode) == GET_MODE_PRECISION (to_mode))
>>> {
>>> if (REAL_MODE_FORMAT (to_mode) == &arm_bfloat_half_format
>>> && REAL_MODE_FORMAT (from_mode) == &ieee_half_format)
>>> /* libgcc implements just __trunchfbf2, not __extendhfbf2. */
>>> tab = trunc_optab;
>>> else
>>> /* Conversion between decimal float and binary float, same
>>> size. */
>>> tab = DECIMAL_FLOAT_MODE_P (from_mode) ? trunc_optab : sext_optab;
>>>
>>> for the new pairing? The intent for bfloat/half seems to be that bfloat
>>> is treated as arbitrarily “lesser than” half, so half->bfloat is a
>>> truncation and bfloat->half is an extension. It seems like it would be
>>> good to do something similar for the new pair, so that the float modes
>>> still form a total order in terms of extension & truncation.
>>
>> Good question! If I read it right, this special handling for half->bfloat is
>> to align with the previous implementation in libgcc and easy for backporting
>> , but isn't to keep the modes to form a total order, as Jakub's comments
>> PR114907 #c6 and #c8. Similar to half vs. bfloat, neither of ibm128 nor
>> ieee128
>> is a subset or superset of the other, the current behavior for this new
>> paring is
>> that:
>> 1) define sext_optab for any two of TF/KF/IF (also bi-direction), since
>> generic
>> code above adopts sext_optab for same size conversion.
>
> But before your patch, that code only expected equal precisions if:
>
> || (DECIMAL_FLOAT_MODE_P (from_mode)
> != DECIMAL_FLOAT_MODE_P (to_mode))
> || (REAL_MODE_FORMAT (from_mode) == &arm_bfloat_half_format
> && REAL_MODE_FORMAT (to_mode) == &ieee_half_format)
> || (REAL_MODE_FORMAT (to_mode) == &arm_bfloat_half_format
> && REAL_MODE_FORMAT (from_mode) == &ieee_half_format)
>
> So the effect was:
>
> binary->decimal: extend
> decimal->binary: truncate
> bfloat->IEEE: extend
> IEEE->bfloat: truncate
>
> AFAICT there was no X and Y for which X->Y and Y->X are both extensions.
Yeah, this is exactly the status quo, though Jakub's comments implied that
IEEE->bfloat should have been supported with extend.
>
>> 2) for each define_expand in 1), it actually calls
>> rs6000_expand_float128_convert
>> which is one existing helper to handle conversions to/from __float128
>> and
>> __ibm128, it will take care of the remaining (generate __extend* or
>> __trunc*).
>> Similar to half vs. bfloat which has extend and trunc libgcc functions,
>> we
>> have some extend and trunc libgcc functions for __float128 vs. __ibm128.
>>
>> We can add some special treatment for the new pairing like what's for half
>> vs. bfloat,
>> but since extend and truncate can be arbitrary for them, I thought we can
>> just define
>> sext_optab to align with what generic code does and wants, then do the
>> actual handling
>> in the corresponding expander (someone may argue that sext_optab is expected
>> to be end
>> with libcall __extend*, it's surprising to end up with __trunc*, but
>> considering
>> the extend/truncate here is arbitrary, guessing it's acceptable?).
>
> Yeah, using a trunc libgcc function for sext_optab seems surprising to me.
OK, thanks for the input! Unfortunately it's even worse in rs6000:
set_conv_libfunc (sext_optab, mode, IFmode, "__trunctfkf2");
if (mode != TFmode && FLOAT128_IBM_P (TFmode))
set_conv_libfunc (sext_optab, mode, TFmode, "__trunctfkf2");
set_conv_libfunc (trunc_optab, IFmode, mode, "__extendkftf2");
if (mode != TFmode && FLOAT128_IBM_P (TFmode))
set_conv_libfunc (trunc_optab, TFmode, mode, "__extendkftf2");
, the optabs and their corresponding libgcc functions look reversed.
I'll follow up to adjust them as well.
>
> Like Jakub said in #c8 above, both extension and truncation are conceptually
> wrong. But given that both are wrong, we can't use the literal meaning to
> choose between them, and need to find something else instead.
>
> Making the sext/trunc choice mirror the current libgcc functions seems
> cleaner. It also avoids the (IMO) odd situation that you can construct
> a valid and infinitely long chain of extensions from a finite set of types.
> IMO the types should be well ordered if possible.
Good point, from this perspective, bidirectional extend seems not recommended.
One updated version has been attached, which checks trunc optab for ibm128 ->
ieee128 similar to bfloat -> ieee16 as we have libgcc function __trunctfkf2
but not __extendtfkf2. Does it look good to you?
I'll post an updated version for rs6000 part once the testing goes well.
BR,
Kewen
-----
From 81f9bb0c40817cbecbff63a9925039de8e6333ff Mon Sep 17 00:00:00 2001
From: Kewen Lin <li...@linux.ibm.com>
Date: Tue, 2 Jul 2024 02:03:55 -0500
Subject: [PATCH 3/4] expr: Allow same precision modes conversion between
{ibm_extended, ieee_quad}_format
With some historical reasons, rs6000 defines KFmode, TFmode
and IFmode to have different mode precisions, but it causes
some issues and needs some workarounds such as PR112993.
So we are going to make all rs6000 128 bit scalar FP modes
have 128 bit precision. Be prepared for that, this patch
is to make function convert_mode_scalar allow same precision
FP modes conversion if their underlying formats are
ibm_extended_format and ieee_quad_format respectively, just
like the existing special treatment on arm_bfloat_half_format
<-> ieee_half_format. It also factors out all the relevant
checks into a lambda function. Besides, similar to ieee fp16
-> bfloat conversion, it adopts trunc_optab rather than
sext_optab for ibm128 to ieee128 conversion.
PR target/112993
gcc/ChangeLog:
* expr.cc (convert_mode_scalar): Allow same precision conversion
between scalar floating point modes if whose underlying format is
ibm_extended_format or ieee_quad_format, and refactor assertion
with new lambda function acceptable_same_precision_modes. Use
trunc_optab rather than sext_optab for ibm128 to ieee128 conversion.
* optabs-libfuncs.cc (gen_trunc_conv_libfunc): Use trunc_optab rather
than sext_optab for ibm128 to ieee128 conversion.
---
gcc/expr.cc | 39 ++++++++++++++++++++++++++++++---------
gcc/optabs-libfuncs.cc | 4 +++-
2 files changed, 33 insertions(+), 10 deletions(-)
diff --git a/gcc/expr.cc b/gcc/expr.cc
index ffbac513692..34679d464e8 100644
--- a/gcc/expr.cc
+++ b/gcc/expr.cc
@@ -338,6 +338,29 @@ convert_mode_scalar (rtx to, rtx from, int unsignedp)
enum rtx_code equiv_code = (unsignedp < 0 ? UNKNOWN
: (unsignedp ? ZERO_EXTEND : SIGN_EXTEND));
+ auto acceptable_same_precision_modes
+ = [] (scalar_mode from_mode, scalar_mode to_mode) -> bool
+ {
+ if (DECIMAL_FLOAT_MODE_P (from_mode) != DECIMAL_FLOAT_MODE_P (to_mode))
+ return true;
+
+ /* arm_bfloat_half_format <-> ieee_half_format */
+ if ((REAL_MODE_FORMAT (from_mode) == &arm_bfloat_half_format
+ && REAL_MODE_FORMAT (to_mode) == &ieee_half_format)
+ || (REAL_MODE_FORMAT (to_mode) == &arm_bfloat_half_format
+ && REAL_MODE_FORMAT (from_mode) == &ieee_half_format))
+ return true;
+
+ /* ibm_extended_format <-> ieee_quad_format */
+ if ((REAL_MODE_FORMAT (from_mode) == &ibm_extended_format
+ && REAL_MODE_FORMAT (to_mode) == &ieee_quad_format)
+ || (REAL_MODE_FORMAT (from_mode) == &ieee_quad_format
+ && REAL_MODE_FORMAT (to_mode) == &ibm_extended_format))
+ return true;
+
+ return false;
+ };
+
if (to_real)
{
rtx value;
@@ -346,18 +369,16 @@ convert_mode_scalar (rtx to, rtx from, int unsignedp)
gcc_assert ((GET_MODE_PRECISION (from_mode)
!= GET_MODE_PRECISION (to_mode))
- || (DECIMAL_FLOAT_MODE_P (from_mode)
- != DECIMAL_FLOAT_MODE_P (to_mode))
- || (REAL_MODE_FORMAT (from_mode) == &arm_bfloat_half_format
- && REAL_MODE_FORMAT (to_mode) == &ieee_half_format)
- || (REAL_MODE_FORMAT (to_mode) == &arm_bfloat_half_format
- && REAL_MODE_FORMAT (from_mode) == &ieee_half_format));
+ || acceptable_same_precision_modes (from_mode, to_mode));
if (GET_MODE_PRECISION (from_mode) == GET_MODE_PRECISION (to_mode))
{
- if (REAL_MODE_FORMAT (to_mode) == &arm_bfloat_half_format
- && REAL_MODE_FORMAT (from_mode) == &ieee_half_format)
- /* libgcc implements just __trunchfbf2, not __extendhfbf2. */
+ if ((REAL_MODE_FORMAT (to_mode) == &arm_bfloat_half_format
+ && REAL_MODE_FORMAT (from_mode) == &ieee_half_format)
+ || (REAL_MODE_FORMAT (to_mode) == &ieee_quad_format
+ && REAL_MODE_FORMAT (from_mode) == &ibm_extended_format))
+ /* libgcc implements just __trunchfbf2, not __extendhfbf2;
+ and __trunctfkf2, not __trunctfkf2. */
tab = trunc_optab;
else
/* Conversion between decimal float and binary float, same
diff --git a/gcc/optabs-libfuncs.cc b/gcc/optabs-libfuncs.cc
index 26729910d92..ab97eace80e 100644
--- a/gcc/optabs-libfuncs.cc
+++ b/gcc/optabs-libfuncs.cc
@@ -591,7 +591,9 @@ gen_trunc_conv_libfunc (convert_optab tab,
if (GET_MODE_PRECISION (float_fmode) <= GET_MODE_PRECISION (float_tmode)
&& (REAL_MODE_FORMAT (float_tmode) != &arm_bfloat_half_format
- || REAL_MODE_FORMAT (float_fmode) != &ieee_half_format))
+ || REAL_MODE_FORMAT (float_fmode) != &ieee_half_format)
+ && (REAL_MODE_FORMAT (float_tmode) != &ieee_quad_format
+ || REAL_MODE_FORMAT (float_fmode) != &ibm_extended_format))
return;
if (GET_MODE_CLASS (float_tmode) == GET_MODE_CLASS (float_fmode))
--
2.43.0
