| Message ID | 1a669daea19f586636a7aba0dce9dd1627ce3459.1717134752.git.linkw@linux.ibm.com |
|---|---|
| State | New |
| Headers | show |
| Series | Replace {FLOAT,{,LONG_}DOUBLE}_TYPE_SIZE with new hook | expand |
Kewen Lin <linkw@linux.ibm.com> writes: > Joseph pointed out "floating types should have their mode, > not a poorly defined precision value" in the discussion[1], > as he and Richi suggested, the existing macros > {FLOAT,{,LONG_}DOUBLE}_TYPE_SIZE will be replaced with a > hook mode_for_floating_type. To be prepared for that, this > patch is to replace use of {FLOAT,{,LONG_}DOUBLE}_TYPE_SIZE > in m2 with TYPE_PRECISION of {float,{,long_}double}_type_node. > > [1] https://gcc.gnu.org/pipermail/gcc-patches/2024-May/651209.html > > gcc/m2/ChangeLog: > > * gm2-gcc/m2type.cc (build_m2_short_real_node): Use TYPE_PRECISION of > float_type_node to replace FLOAT_TYPE_SIZE. > (build_m2_real_node): Use TYPE_PRECISION of double_type_node to > replace DOUBLE_TYPE_SIZE. > (build_m2_long_real_node): Use TYPE_PRECISION of > long_double_type_node to replace LONG_DOUBLE_TYPE_SIZE. > --- > gcc/m2/gm2-gcc/m2type.cc | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > 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); > } > else if (M2Options_GetIEEELongDouble ()) > longreal = float128_type_node; all look good to me thanks, regards, Gaius
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. (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.)
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: 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? > > (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; unless there is some special need requiring one different node for ibmlongdouble, then: diff --git a/gcc/m2/gm2-gcc/m2type.cc b/gcc/m2/gm2-gcc/m2type.cc index c808e3e8dbb..5e479e4bbce 100644 --- a/gcc/m2/gm2-gcc/m2type.cc +++ b/gcc/m2/gm2-gcc/m2type.cc @@ -1450,6 +1450,7 @@ build_m2_long_real_node (void) { longreal = make_node (REAL_TYPE); TYPE_PRECISION (longreal) = TYPE_PRECISION (long_double_type_node); + SET_TYPE_MODE (longreal, TYPE_MODE (long_double_type_node)); } else if (M2Options_GetIEEELongDouble ()) longreal = float128_type_node; BR, Kewen
"Kewen.Lin" <linkw@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
Hi Gaius, on 2024/6/5 22:22, Gaius Mulley wrote: > "Kewen.Lin" <linkw@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. ah, ok, thanks for clarifying! > >>> (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: Nice! Looking forward to you pushing this new one (I'm withdrawing the original patch). > > 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); It looks that float_type_node, double_type_node, float128_type_node and long_double_type_node have been called with layout_type when they are being initialized in function build_common_tree_nodes, maybe we can just assert their TYPE_SIZE. BR, Kewen > + 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 >
"Kewen.Lin" <linkw@linux.ibm.com> writes: Hi Kewen, > Nice! Looking forward to you pushing this new one (I'm withdrawing the original > patch). all pushed now - thanks for the original patch! regards, Gaius
Hi Gaius, >> 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); > > It looks that float_type_node, double_type_node, float128_type_node and > long_double_type_node have been called with layout_type when they are > being initialized in function build_common_tree_nodes, maybe we can just > assert their TYPE_SIZE. I just noticed that latest trunk still has {FLOAT,{,LONG_}DOUBLE}_TYPE_SIZE in gcc/m2 and realized that my comment above was misleading, sorry about that. It meant TYPE_SIZE (float_type_node) etc. instead of {FLOAT,{,LONG_}DOUBLE}_TYPE_SIZE, as this patch series would like to get rid of {FLOAT,{,LONG_}DOUBLE}_TYPE_SIZE. I adjusted them as below patch, does this look good to you? BR, Kewen ----- [PATCH] m2: Remove uses of {FLOAT,{,LONG_}DOUBLE}_TYPE_SIZE Joseph pointed out "floating types should have their mode, not a poorly defined precision value" in the discussion[1], as he and Richi suggested, the existing macros {FLOAT,{,LONG_}DOUBLE}_TYPE_SIZE will be replaced with a hook mode_for_floating_type. To be prepared for that, this patch is to remove uses of {FLOAT,{,LONG_}DOUBLE}_TYPE_SIZE in m2. Currently they are used for assertion and can be replaced with TYPE_SIZE check on the corresponding type node, since we dropped the call to layout_type which would early return once TYPE_SIZE is set and this assertion ensures it's safe to drop that call. [1] https://gcc.gnu.org/pipermail/gcc-patches/2024-May/651209.html gcc/m2/ChangeLog: * gm2-gcc/m2type.cc (build_m2_short_real_node): Adjust assertion with TYPE_SIZE check. (build_m2_real_node): Likewise. (build_m2_long_real_node): Add assertion with TYPE_SIZE check. --- gcc/m2/gm2-gcc/m2type.cc | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/gcc/m2/gm2-gcc/m2type.cc b/gcc/m2/gm2-gcc/m2type.cc index 5773a5cbd19..7ed184518cb 100644 --- a/gcc/m2/gm2-gcc/m2type.cc +++ b/gcc/m2/gm2-gcc/m2type.cc @@ -1416,7 +1416,7 @@ static tree build_m2_short_real_node (void) { /* Define `SHORTREAL'. */ - ASSERT_CONDITION (TYPE_PRECISION (float_type_node) == FLOAT_TYPE_SIZE); + ASSERT_CONDITION (TYPE_SIZE (float_type_node)); return float_type_node; } @@ -1424,7 +1424,7 @@ static tree build_m2_real_node (void) { /* Define `REAL'. */ - ASSERT_CONDITION (TYPE_PRECISION (double_type_node) == DOUBLE_TYPE_SIZE); + ASSERT_CONDITION (TYPE_SIZE (double_type_node)); return double_type_node; } @@ -1432,12 +1432,13 @@ static tree build_m2_long_real_node (void) { tree longreal; - + /* Define `LONGREAL'. */ if (M2Options_GetIEEELongDouble ()) longreal = float128_type_node; else longreal = long_double_type_node; + ASSERT_CONDITION (TYPE_SIZE (longreal)); return longreal; } -- 2.43.0
"Kewen.Lin" <linkw@linux.ibm.com> writes: > Hi Gaius, > >>> 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); >> >> It looks that float_type_node, double_type_node, float128_type_node and >> long_double_type_node have been called with layout_type when they are >> being initialized in function build_common_tree_nodes, maybe we can just >> assert their TYPE_SIZE. > > I just noticed that latest trunk still has {FLOAT,{,LONG_}DOUBLE}_TYPE_SIZE > in gcc/m2 and realized that my comment above was misleading, sorry about that. > It meant TYPE_SIZE (float_type_node) etc. instead of {FLOAT,{,LONG_}DOUBLE}_TYPE_SIZE, > as this patch series would like to get rid of {FLOAT,{,LONG_}DOUBLE}_TYPE_SIZE. > > I adjusted them as below patch, does this look good to you? Hi Kewen, ah yes indeed, lgtm, regards, Gaius > > BR, > Kewen > ----- > > [PATCH] m2: Remove uses of {FLOAT,{,LONG_}DOUBLE}_TYPE_SIZE > > Joseph pointed out "floating types should have their mode, > not a poorly defined precision value" in the discussion[1], > as he and Richi suggested, the existing macros > {FLOAT,{,LONG_}DOUBLE}_TYPE_SIZE will be replaced with a > hook mode_for_floating_type. To be prepared for that, this > patch is to remove uses of {FLOAT,{,LONG_}DOUBLE}_TYPE_SIZE > in m2. Currently they are used for assertion and can be > replaced with TYPE_SIZE check on the corresponding type node, > since we dropped the call to layout_type which would early > return once TYPE_SIZE is set and this assertion ensures it's > safe to drop that call. > > [1] https://gcc.gnu.org/pipermail/gcc-patches/2024-May/651209.html > > gcc/m2/ChangeLog: > > * gm2-gcc/m2type.cc (build_m2_short_real_node): Adjust assertion with > TYPE_SIZE check. > (build_m2_real_node): Likewise. > (build_m2_long_real_node): Add assertion with TYPE_SIZE check. > --- > gcc/m2/gm2-gcc/m2type.cc | 7 ++++--- > 1 file changed, 4 insertions(+), 3 deletions(-) > > diff --git a/gcc/m2/gm2-gcc/m2type.cc b/gcc/m2/gm2-gcc/m2type.cc > index 5773a5cbd19..7ed184518cb 100644 > --- a/gcc/m2/gm2-gcc/m2type.cc > +++ b/gcc/m2/gm2-gcc/m2type.cc > @@ -1416,7 +1416,7 @@ static tree > build_m2_short_real_node (void) > { > /* Define `SHORTREAL'. */ > - ASSERT_CONDITION (TYPE_PRECISION (float_type_node) == FLOAT_TYPE_SIZE); > + ASSERT_CONDITION (TYPE_SIZE (float_type_node)); > return float_type_node; > } > > @@ -1424,7 +1424,7 @@ static tree > build_m2_real_node (void) > { > /* Define `REAL'. */ > - ASSERT_CONDITION (TYPE_PRECISION (double_type_node) == DOUBLE_TYPE_SIZE); > + ASSERT_CONDITION (TYPE_SIZE (double_type_node)); > return double_type_node; > } > > @@ -1432,12 +1432,13 @@ static tree > build_m2_long_real_node (void) > { > tree longreal; > - > + > /* Define `LONGREAL'. */ > if (M2Options_GetIEEELongDouble ()) > longreal = float128_type_node; > else > longreal = long_double_type_node; > + ASSERT_CONDITION (TYPE_SIZE (longreal)); > return longreal; > } > > -- > 2.43.0
on 2024/6/15 13:00, Gaius Mulley wrote: > "Kewen.Lin" <linkw@linux.ibm.com> writes: > >> Hi Gaius, >> >>>> 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); >>> >>> It looks that float_type_node, double_type_node, float128_type_node and >>> long_double_type_node have been called with layout_type when they are >>> being initialized in function build_common_tree_nodes, maybe we can just >>> assert their TYPE_SIZE. >> >> I just noticed that latest trunk still has {FLOAT,{,LONG_}DOUBLE}_TYPE_SIZE >> in gcc/m2 and realized that my comment above was misleading, sorry about that. >> It meant TYPE_SIZE (float_type_node) etc. instead of {FLOAT,{,LONG_}DOUBLE}_TYPE_SIZE, >> as this patch series would like to get rid of {FLOAT,{,LONG_}DOUBLE}_TYPE_SIZE. >> >> I adjusted them as below patch, does this look good to you? > > Hi Kewen, > > ah yes indeed, lgtm, Thanks Gaius! Pushed as r15-1362-g96fe23eb8a9eba. BR, Kewen
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); } else if (M2Options_GetIEEELongDouble ()) longreal = float128_type_node;
Joseph pointed out "floating types should have their mode, not a poorly defined precision value" in the discussion[1], as he and Richi suggested, the existing macros {FLOAT,{,LONG_}DOUBLE}_TYPE_SIZE will be replaced with a hook mode_for_floating_type. To be prepared for that, this patch is to replace use of {FLOAT,{,LONG_}DOUBLE}_TYPE_SIZE in m2 with TYPE_PRECISION of {float,{,long_}double}_type_node. [1] https://gcc.gnu.org/pipermail/gcc-patches/2024-May/651209.html gcc/m2/ChangeLog: * gm2-gcc/m2type.cc (build_m2_short_real_node): Use TYPE_PRECISION of float_type_node to replace FLOAT_TYPE_SIZE. (build_m2_real_node): Use TYPE_PRECISION of double_type_node to replace DOUBLE_TYPE_SIZE. (build_m2_long_real_node): Use TYPE_PRECISION of long_double_type_node to replace LONG_DOUBLE_TYPE_SIZE. --- gcc/m2/gm2-gcc/m2type.cc | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)