"Kewen.Lin" <li...@linux.ibm.com> writes:
> Hi Joseph and Gaius,
>
> on 2024/6/4 02:02, Joseph Myers wrote:
>> On Sun, 2 Jun 2024, Kewen Lin wrote:
>>
>>> diff --git a/gcc/m2/gm2-gcc/m2type.cc b/gcc/m2/gm2-gcc/m2type.cc
>>> index 571923c08ef..d52cbdf0b99 100644
>>> --- a/gcc/m2/gm2-gcc/m2type.cc
>>> +++ b/gcc/m2/gm2-gcc/m2type.cc
>>> @@ -1420,7 +1420,7 @@ build_m2_short_real_node (void)
>>> /* Define `REAL'. */
>>>
>>> c = make_node (REAL_TYPE);
>>> - TYPE_PRECISION (c) = FLOAT_TYPE_SIZE;
>>> + TYPE_PRECISION (c) = TYPE_PRECISION (float_type_node);
>>> layout_type (c);
>>> return c;
>>> }
>>> @@ -1433,7 +1433,7 @@ build_m2_real_node (void)
>>> /* Define `REAL'. */
>>>
>>> c = make_node (REAL_TYPE);
>>> - TYPE_PRECISION (c) = DOUBLE_TYPE_SIZE;
>>> + TYPE_PRECISION (c) = TYPE_PRECISION (double_type_node);
>>> layout_type (c);
>>> return c;
>>> }
>>> @@ -1447,7 +1447,7 @@ build_m2_long_real_node (void)
>>> if (M2Options_GetIBMLongDouble ())
>>> {
>>> longreal = make_node (REAL_TYPE);
>>> - TYPE_PRECISION (longreal) = LONG_DOUBLE_TYPE_SIZE;
>>> + TYPE_PRECISION (longreal) = TYPE_PRECISION (long_double_type_node);
>>
>> This looks rather like m2 would still have the same problem the generic
>> code previously had: going via precision when that might not uniquely
>> determine the desired machine mode. And so making sure to use the right
>> machine mode as done when setting up long_double_type_node etc. would be
>> better than keeping this code copying TYPE_PRECISION and hoping to
>> determine a machine mode from that. It certainly looks like this code
>> wants to match float, double and long double, rather than possibly getting
>> a different mode with possibly the same TYPE_PRECISION.
>
> Good point, sorry that I just did a replacement without checking the context.
> If the above holds (Gaius can confirm or clarify), SET_TYPE_MODE would be
> also applied here, that is:
Hi Kewen and Joseph,
yes the code is attempting to create nodes SHORTREAL, REAL, LONGREAL
mapping onto the C float, double, long double nodes.
> diff --git a/gcc/m2/gm2-gcc/m2type.cc b/gcc/m2/gm2-gcc/m2type.cc
> index d52cbdf0b99..5ff02a18876 100644
> --- a/gcc/m2/gm2-gcc/m2type.cc
> +++ b/gcc/m2/gm2-gcc/m2type.cc
> @@ -1421,6 +1421,7 @@ build_m2_short_real_node (void)
>
> c = make_node (REAL_TYPE);
> TYPE_PRECISION (c) = TYPE_PRECISION (float_type_node);
> + SET_TYPE_MODE (c, TYPE_MODE (float_type_node));
> layout_type (c);
> return c;
> }
> @@ -1434,6 +1435,7 @@ build_m2_real_node (void)
>
> c = make_node (REAL_TYPE);
> TYPE_PRECISION (c) = TYPE_PRECISION (double_type_node);
> + SET_TYPE_MODE (c, TYPE_MODE (double_type_node));
> layout_type (c);
> return c;
> }
>
> I'm not sure and curious why the above builds new nodes for short real and
> real but re-use float128_type_node or long_double_type_node for some cases,
> some special needs cause the former ones should have separated nodes?
good point - there is no need to create new nodes.
>> (I don't know if the M2Options_GetIBMLongDouble call would be needed at
>> all once you use the machine mode for long double in a reliable way, or
>> whether this code could be further simplified.)
>
> long_double_type_node should already take care of ibmlongdouble, IIUC it
> would be like:
>
> @@ -1443,13 +1445,7 @@ build_m2_long_real_node (void)
> {
> tree longreal;
>
> - /* Define `LONGREAL'. */
> - if (M2Options_GetIBMLongDouble ())
> - {
> - longreal = make_node (REAL_TYPE);
> - TYPE_PRECISION (longreal) = TYPE_PRECISION (long_double_type_node);
> - }
> - else if (M2Options_GetIEEELongDouble ())
> + if (M2Options_GetIEEELongDouble ())
> longreal = float128_type_node;
> else
> longreal = long_double_type_node;
yes indeed and the above patch is fine, all bootstrapped and regression
tested on ppc64le (cfarm120). I've also bootstrapped and regression tested
the following patch on ppc64le and aarch64:
diff --git a/gcc/m2/gm2-gcc/m2type.cc b/gcc/m2/gm2-gcc/m2type.cc
index 571923c08ef..ce53130e2a9 100644
--- a/gcc/m2/gm2-gcc/m2type.cc
+++ b/gcc/m2/gm2-gcc/m2type.cc
@@ -1415,41 +1415,26 @@ build_m2_char_node (void)
static tree
build_m2_short_real_node (void)
{
- tree c;
-
- /* Define `REAL'. */
-
- c = make_node (REAL_TYPE);
- TYPE_PRECISION (c) = FLOAT_TYPE_SIZE;
- layout_type (c);
- return c;
+ /* Define `SHORTREAL'. */
+ layout_type (float_type_node);
+ return float_type_node;
}
static tree
build_m2_real_node (void)
{
- tree c;
-
/* Define `REAL'. */
-
- c = make_node (REAL_TYPE);
- TYPE_PRECISION (c) = DOUBLE_TYPE_SIZE;
- layout_type (c);
- return c;
+ layout_type (double_type_node);
+ return double_type_node;
}
static tree
build_m2_long_real_node (void)
{
tree longreal;
-
+
/* Define `LONGREAL'. */
- if (M2Options_GetIBMLongDouble ())
- {
- longreal = make_node (REAL_TYPE);
- TYPE_PRECISION (longreal) = LONG_DOUBLE_TYPE_SIZE;
- }
- else if (M2Options_GetIEEELongDouble ())
+ if (M2Options_GetIEEELongDouble ())
longreal = float128_type_node;
else
longreal = long_double_type_node;
regards,
Gaius
