Re: [RFC/PATCH] Replace {FLOAT,{,LONG_}DOUBLE}_TYPE_SIZE with new hook
On Wed, 29 May 2024, Kewen.Lin wrote: > > Note that when removing a target macro, it's a good idea to add it to the > > "Old target macros that have moved to the target hooks structure." list > > (of #pragma GCC poison) in system.h to ensure any new target that was > > originally written before the change doesn't accidentally get into GCC > > while still using the old macros. > > > > Thanks for the comments on target macro removal! I found it means > that we can't use such macros any more even if they have become port > specific. For some targets such as pa, they redefine these macros in Yes, that's intentional. If you need subtarget headers to define something for use in the new target hook for those targets, make them define e.g. SUBTARGET_LONG_DOUBLE_MODE or similar instead, rather than using the old poisoned macro names. -- Joseph S. Myers josmy...@redhat.com
Re: [RFC/PATCH] Replace {FLOAT,{,LONG_}DOUBLE}_TYPE_SIZE with new hook
Hi Richi and Joseph, on 2024/5/24 20:23, Richard Biener wrote: > On Fri, May 24, 2024 at 12:20 PM Kewen.Lin wrote: >> btw, the attached patch is bootstrapped and regtested on >> powerpc64-linux-gnu and powerpc64le-linux-gnu with all >> languages on, cross cc1 built well for affected ports. > > Looks reasonable to me - I'd split language changes out but > keep target and middle-end together. The middle-end parts > look good to me - I'm always a bit nervous when using > size and precision exchangably, esp. for FP, but it seems > this has been done before. Thanks for the suggestion! I'll split them into a patch series as components soon and follow this suggestion when committing (some preparation language changes go first and squash the others together). on 2024/5/29 05:06, Joseph Myers wrote: > On Fri, 24 May 2024, Kewen.Lin wrote: > >> Following your suggestion and comments, I made this patch >> for mode_for_floating_type first, considering this touches >> a few FE and port specific code, I think I have to split >> it into a patch series. Before making that, I'd like to >> ensure this meets what you expected, and also seek for the > > The general idea seems reasonable (I haven't reviewed it in detail). > Note that when removing a target macro, it's a good idea to add it to the > "Old target macros that have moved to the target hooks structure." list > (of #pragma GCC poison) in system.h to ensure any new target that was > originally written before the change doesn't accidentally get into GCC > while still using the old macros. > Thanks for the comments on target macro removal! I found it means that we can't use such macros any more even if they have become port specific. For some targets such as pa, they redefine these macros in some subtarget headers, or these macros get used in other macro definitions. Considering leaving them can have better readability, I didn't try to change them in this RFC/PATCH, I'll update them with target prefix in the following patch series. BR, Kewen
Re: [RFC/PATCH] Replace {FLOAT,{,LONG_}DOUBLE}_TYPE_SIZE with new hook
On Fri, 24 May 2024, Kewen.Lin wrote: > Following your suggestion and comments, I made this patch > for mode_for_floating_type first, considering this touches > a few FE and port specific code, I think I have to split > it into a patch series. Before making that, I'd like to > ensure this meets what you expected, and also seek for the The general idea seems reasonable (I haven't reviewed it in detail). Note that when removing a target macro, it's a good idea to add it to the "Old target macros that have moved to the target hooks structure." list (of #pragma GCC poison) in system.h to ensure any new target that was originally written before the change doesn't accidentally get into GCC while still using the old macros. -- Joseph S. Myers josmy...@redhat.com
Re: [RFC/PATCH] Replace {FLOAT, {, LONG_}DOUBLE}_TYPE_SIZE with new hook
On Fri, May 24, 2024 at 12:20 PM Kewen.Lin wrote: > > Hi Joseph and Richi, > > on 2024/5/13 21:18, Joseph Myers wrote: > > On Mon, 13 May 2024, Kewen.Lin wrote: > > > >>> In fact replacing all of X_TYPE_SIZE with a single hook might be > >>> worthwhile > >>> though this removes the "convenient" defaulting, requiring each target to > >>> enumerate all standard C ABI type modes. But that might be also a good > >>> thing. > >>> > >> > >> I guess the main value by extending from floating point types to all is to > >> unify them? (Assuming that excepting for floating types the others would > >> not have multiple possible representations like what we faces on 128bit > >> fp). > > > > For integer types, giving the number of bits makes sense as an interface - > > there isn't an issue with different modes. > > > > So I think it's appropriate for floating and integer types to have > > separate hooks - with the one for floating types returning a mode, and the > > one for integer types returning a number of bits. (And also keep the > > existing separate hook for _FloatN / _FloatNx modes.) > > > > That may also make for more convenient defaults (whether a target has long > > double wider than double is largely independent of what sizes it uses for > > integer types). > > > > Following your suggestion and comments, I made this patch > for mode_for_floating_type first, considering this touches > a few FE and port specific code, I think I have to split > it into a patch series. Before making that, I'd like to > ensure this meets what you expected, and also seek for the > suggestion on how to organize the sub-patches. There seem > two ways for sub-patches: > 1) split this into pieces according to FEs and ports, and > squash all of them and commit one patch. > 2) extract all hook implementation as 1st series (split > as ports); > extract the hook enablement as 2nd part (split as > generic and FEs); > the remaining is to remove useless macros (split it > as generic and ports); > > The 1) is straightforward, while the 2) is fine-grained and > easy for isolation, but not sure if it's worth doing. > > btw, the attached patch is bootstrapped and regtested on > powerpc64-linux-gnu and powerpc64le-linux-gnu with all > languages on, cross cc1 built well for affected ports. Looks reasonable to me - I'd split language changes out but keep target and middle-end together. The middle-end parts look good to me - I'm always a bit nervous when using size and precision exchangably, esp. for FP, but it seems this has been done before. I hope Joseph will eye that part as well. Thanks for doing this, Richard. > BR, > Kewen > > -
[RFC/PATCH] Replace {FLOAT,{,LONG_}DOUBLE}_TYPE_SIZE with new hook
Hi Joseph and Richi,
on 2024/5/13 21:18, Joseph Myers wrote:
> On Mon, 13 May 2024, Kewen.Lin wrote:
>
>>> In fact replacing all of X_TYPE_SIZE with a single hook might be worthwhile
>>> though this removes the "convenient" defaulting, requiring each target to
>>> enumerate all standard C ABI type modes. But that might be also a good
>>> thing.
>>>
>>
>> I guess the main value by extending from floating point types to all is to
>> unify them? (Assuming that excepting for floating types the others would
>> not have multiple possible representations like what we faces on 128bit fp).
>
> For integer types, giving the number of bits makes sense as an interface -
> there isn't an issue with different modes.
>
> So I think it's appropriate for floating and integer types to have
> separate hooks - with the one for floating types returning a mode, and the
> one for integer types returning a number of bits. (And also keep the
> existing separate hook for _FloatN / _FloatNx modes.)
>
> That may also make for more convenient defaults (whether a target has long
> double wider than double is largely independent of what sizes it uses for
> integer types).
>
Following your suggestion and comments, I made this patch
for mode_for_floating_type first, considering this touches
a few FE and port specific code, I think I have to split
it into a patch series. Before making that, I'd like to
ensure this meets what you expected, and also seek for the
suggestion on how to organize the sub-patches. There seem
two ways for sub-patches:
1) split this into pieces according to FEs and ports, and
squash all of them and commit one patch.
2) extract all hook implementation as 1st series (split
as ports);
extract the hook enablement as 2nd part (split as
generic and FEs);
the remaining is to remove useless macros (split it
as generic and ports);
The 1) is straightforward, while the 2) is fine-grained and
easy for isolation, but not sure if it's worth doing.
btw, the attached patch is bootstrapped and regtested on
powerpc64-linux-gnu and powerpc64le-linux-gnu with all
languages on, cross cc1 built well for affected ports.
BR,
Kewen
-
From 2935750160f4eaf72eb7fba5832c99d6bf552862 Mon Sep 17 00:00:00 2001
From: Kewen Lin
Date: Fri, 24 May 2024 00:10:22 -0500
Subject: [PATCH] Replace {FLOAT,{,LONG_}DOUBLE}_TYPE_SIZE with new hook
mode_for_floating_type
Currently how we determine which mode will be used for a
floating point type is that for a given type precision
(size) call mode_for_size to get the first mode which has
this size in the specified class. On Powerpc, we have
three modes (TF/KF/IF) having the same mode precision 128
(see[1]), so the processing forces us to have to place TF
at the first place, it would require us to make more
adjustment in some generic code to avoid some unexpected
mode conversions and it would be even worse if we get rid
of TF eventually one day. And as Joseph pointed out in [2],
"floating types should have their mode, not a poorly
defined precision value", as Joseph and Richi suggested,
this patch is to introduce one hook mode_for_floating_type
which returns the corresponding mode for type float, double
or long double. The default implementation returns SFmode
for float and DFmode for double or long double, and ports
which need special treatment have their own port specific
implementation (referring to {,LONG_}DOUBLE_TYPE_SIZE).
For all generic uses of {FLOAT,{,LONG_}DOUBLE}_TYPE_SIZE,
depending on the context, it replaces them with
TYPE_PRECISION of the according type node, or
GET_MODE_PRECISION on the mode from mode_for_floating_type.
It also removes some useless uses of
{FLOAT,{,LONG_}DOUBLE}_TYPE_SIZE in target specific codes,
but leaves those being used (like defining other macros)
untouched.
[1] https://gcc.gnu.org/pipermail/gcc-patches/2024-May/651017.html
[2] https://gcc.gnu.org/pipermail/gcc-patches/2024-May/651209.html
gcc/ada/ChangeLog:
* gcc-interface/decl.cc (gnat_to_gnu_entity): Use TYPE_PRECISION of
long_double_type_node to replace LONG_DOUBLE_TYPE_SIZE.
gcc/ChangeLog:
* config/aarch64/aarch64.cc (aarch64_c_mode_for_floating_type):
New function.
(TARGET_C_MODE_FOR_FLOATING_TYPE): New macro.
* config/aarch64/aarch64.h (FLOAT_TYPE_SIZE): Remove.
(DOUBLE_TYPE_SIZE): Likewise.
(LONG_DOUBLE_TYPE_SIZE): Likewise.
* config/alpha/alpha.cc (alpha_c_mode_for_floating_type): New
function.
(TARGET_C_MODE_FOR_FLOATING_TYPE): New macro.
* config/alpha/alpha.h (FLOAT_TYPE_SIZE): Remove.
(DOUBLE_TYPE_SIZE): Likewise.
(LONG_DOUBLE_TYPE_SIZE): Likewise.
* config/arc/arc.h (FLOAT_TYPE_SIZE): Remove.
(DOUBLE_TYPE_SIZE): Likewise.
(LONG_DOUBLE_TYPE_SIZE): Likewise.
* config/avr/avr.cc (avr_c_mode_for_floating_type): New
function.
(TARGET_C_MODE_FOR_FLOATING_TYPE): New macro.
*
