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
