Re: [PATCH 2/3] Make __float128 use the _Float128 type, PR target/107299

["Kewen.Lin" <linkw@linux.ibm.com>]

on 2022/12/6 19:27, Kewen.Lin via Gcc-patches wrote:
> Hi Mike,
> 
> Thanks for fixing this, some comments are inlined below.
> 
> on 2022/11/2 10:42, Michael Meissner wrote:
>> This patch fixes the issue that GCC cannot build when the default long double
>> is IEEE 128-bit.  It fails in building libgcc, specifically when it is trying
>> to buld the __mulkc3 function in libgcc.  It is failing in gimple-range-fold.cc
>> during the evrp pass.  Ultimately it is failing because the code declared the
>> type to use TFmode but it used F128 functions (i.e. KFmode).

By further looking into this, I found that though __float128 and _Float128 types
are two different types, they have the same mode TFmode, the unexpected thing is
these two types have different precision.  I noticed it's due to the "workaround"
in build_common_tree_nodes:

      /* Work around the rs6000 KFmode having precision 113 not
	 128.  */
      const struct real_format *fmt = REAL_MODE_FORMAT (mode);
      gcc_assert (fmt->b == 2 && fmt->emin + fmt->emax == 3);
      int min_precision = fmt->p + ceil_log2 (fmt->emax - fmt->emin);
      if (!extended)
	gcc_assert (min_precision == n);
      if (precision < min_precision)
	precision = min_precision;

Since function useless_type_conversion_p considers two float types are compatible
if they have the same mode, so it doesn't require the explicit conversions between
these two types.  I think it's exactly what we want.  And to me, it looks unexpected
to have two types with the same mode but different precision.

So could we consider disabling the above workaround to make _Float128 have the same
precision as __float128 (long double) (the underlying TFmode)?  I tried the below
change:

diff --git a/gcc/tree.cc b/gcc/tree.cc
index 254b2373dcf..10fcb3d88ca 100644
--- a/gcc/tree.cc
+++ b/gcc/tree.cc
@@ -9442,6 +9442,7 @@ build_common_tree_nodes (bool signed_char)
       if (!targetm.floatn_mode (n, extended).exists (&mode))
         continue;
       int precision = GET_MODE_PRECISION (mode);
+#if 0
       /* Work around the rs6000 KFmode having precision 113 not
          128.  */
       const struct real_format *fmt = REAL_MODE_FORMAT (mode);
@@ -9451,6 +9452,7 @@ build_common_tree_nodes (bool signed_char)
         gcc_assert (min_precision == n);
       if (precision < min_precision)
         precision = min_precision;
+#endif
       FLOATN_NX_TYPE_NODE (i) = make_node (REAL_TYPE);
       TYPE_PRECISION (FLOATN_NX_TYPE_NODE (i)) = precision;
       layout_type (FLOATN_NX_TYPE_NODE (i));

It can be bootstrapped (fixing the ICE in PR107299).  Comparing with the baseline
regression test results with patch #1, #2 and #3, I got some positive:

FAIL->PASS: 17_intro/headers/c++2020/all_attributes.cc (test for excess errors)
FAIL->PASS: 17_intro/headers/c++2020/all_no_exceptions.cc (test for excess errors)
FAIL->PASS: 17_intro/headers/c++2020/all_no_rtti.cc (test for excess errors)
FAIL->PASS: 17_intro/headers/c++2020/all_pedantic_errors.cc (test for excess errors)
FAIL->PASS: 17_intro/headers/c++2020/operator_names.cc (test for excess errors)
FAIL->PASS: 17_intro/headers/c++2020/stdc++.cc (test for excess errors)
FAIL->PASS: 17_intro/headers/c++2020/stdc++_multiple_inclusion.cc (test for excess errors)
FAIL->PASS: std/format/arguments/args.cc (test for excess errors)
FAIL->PASS: std/format/error.cc (test for excess errors)
FAIL->PASS: std/format/formatter/requirements.cc (test for excess errors)
FAIL->PASS: std/format/functions/format.cc (test for excess errors)
FAIL->PASS: std/format/functions/format_to_n.cc (test for excess errors)
FAIL->PASS: std/format/functions/size.cc (test for excess errors)
FAIL->PASS: std/format/functions/vformat_to.cc (test for excess errors)
FAIL->PASS: std/format/parse_ctx.cc (test for excess errors)
FAIL->PASS: std/format/string.cc (test for excess errors)
FAIL->PASS: std/format/string_neg.cc (test for excess errors)
FAIL->PASS: g++.dg/cpp23/ext-floating1.C  -std=gnu++23 (test for excess errors)

and some negative:

PASS->FAIL: gcc.dg/torture/float128-nan.c   -O0  execution test
PASS->FAIL: gcc.dg/torture/float128-nan.c   -O1  execution test
PASS->FAIL: gcc.dg/torture/float128-nan.c   -O2  execution test
PASS->FAIL: gcc.dg/torture/float128-nan.c   -O2 -flto -fno-use-linker-plugin -flto-partition=none  execution test
PASS->FAIL: gcc.dg/torture/float128-nan.c   -O2 -flto -fuse-linker-plugin -fno-fat-lto-objects  execution test
PASS->FAIL: gcc.dg/torture/float128-nan.c   -O3 -g  execution test
PASS->FAIL: gcc.dg/torture/float128-nan.c   -Os  execution test
PASS->FAIL: gcc.target/powerpc/nan128-1.c execution test

The negative part is about nan, I haven't looked into it, but I think it may be the
reason why we need the workaround there, CC Joseph.  Anyway it needs more investigation
here, but IMHO the required information (ie. the actual precision) can be retrieved
from REAL_MODE_FORMAT(mode) of TYPE_MODE, so it should be doable to fix some other
places.

Some concerns on the way of patch #2 are:
  1) long double is with TFmode, it's not taken as compatible with _Float128 even
     if -mabi=ieeelongdouble specified, we can have inefficient IRs than before.
  2) we make type attribute with TFmode _Float128 (KFmode), but there is one actual
     type long double with TFmode, they are actually off.

Comparing with patch #2, this way to remove the workaround in build_common_tree_nodes
can still keep the things as before.  It may be something we can consider here.

BR,
Kewen