Re: [PATCH 1/4] rs6000: Make all 128 bit scalar FP modes have 128 bit precision [PR112993]

2024-05-14 Thread Kewen.Lin
Hi Joseph and Richi,

Thanks for the suggestions and comments!

on 2024/5/10 14:31, Richard Biener wrote:
> On Thu, May 9, 2024 at 9:12 PM Joseph Myers  wrote:
>>
>> On Wed, 8 May 2024, Kewen.Lin wrote:
>>
>>> to widen IFmode to TFmode.  To make build_common_tree_nodes
>>> be able to find the correct mode for long double type node,
>>> it introduces one hook mode_for_longdouble to offer target
>>> a way to specify the mode used for long double type node.
>>
>> I don't really like layering a hook on top of the old target macro as a
>> way to address a deficiency in the design of that target macro (floating
>> types should have their mode, not a poorly defined precision value,
>> specified directly by the target).

Good point!

> 
> Seconded.
> 
>> A better hook design might be something like mode_for_floating_type (enum
>> tree_index), where the argument is TI_FLOAT_TYPE, TI_DOUBLE_TYPE or
>> TI_LONG_DOUBLE_TYPE, replacing all definitions and uses of
>> FLOAT_TYPE_SIZE, DOUBLE_TYPE_SIZE and LONG_DOUBLE_TYPE_SIZE with the
>> single new hook and appropriate definitions for each target (with a
>> default definition that uses SFmode for float and DFmode for double and
>> long double, which would be suitable for many targets).
> 

The originally proposed hook was meant to make the other ports unaffected,
but I agree that introducing such hook would be more clear.

> 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).

> The most pragmatic solution would be to do
> s/LONG_DOUBLE_TYPE_SIZE/LONG_DOUBLE_TYPE_MODE/

Yeah, this beats my proposed hook (assuming the default is VOIDmode too).

So it seems we have three alternatives here:
  1) s/LONG_DOUBLE_TYPE_SIZE/LONG_DOUBLE_TYPE_MODE/
  2) mode_for_floating_type
  3) mode_for_abi_type

Since 1) would make long double type special (different from the other types
having _TYPE_SIZE), personally I'm inclined to 3): implement 2) first, get
this patch series landed, extend to all.

Do you have any preference?  

BR,
Kewen


Re: [PATCH 1/4] rs6000: Make all 128 bit scalar FP modes have 128 bit precision [PR112993]

2024-05-13 Thread Joseph Myers
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).

-- 
Joseph S. Myers
josmy...@redhat.com



Re: [PATCH 1/4] rs6000: Make all 128 bit scalar FP modes have 128 bit precision [PR112993]

2024-05-13 Thread Richard Biener
On Mon, May 13, 2024 at 3:39 AM Kewen.Lin  wrote:
>
> Hi Joseph and Richi,
>
> Thanks for the suggestions and comments!
>
> on 2024/5/10 14:31, Richard Biener wrote:
> > On Thu, May 9, 2024 at 9:12 PM Joseph Myers  wrote:
> >>
> >> On Wed, 8 May 2024, Kewen.Lin wrote:
> >>
> >>> to widen IFmode to TFmode.  To make build_common_tree_nodes
> >>> be able to find the correct mode for long double type node,
> >>> it introduces one hook mode_for_longdouble to offer target
> >>> a way to specify the mode used for long double type node.
> >>
> >> I don't really like layering a hook on top of the old target macro as a
> >> way to address a deficiency in the design of that target macro (floating
> >> types should have their mode, not a poorly defined precision value,
> >> specified directly by the target).
>
> Good point!
>
> >
> > Seconded.
> >
> >> A better hook design might be something like mode_for_floating_type (enum
> >> tree_index), where the argument is TI_FLOAT_TYPE, TI_DOUBLE_TYPE or
> >> TI_LONG_DOUBLE_TYPE, replacing all definitions and uses of
> >> FLOAT_TYPE_SIZE, DOUBLE_TYPE_SIZE and LONG_DOUBLE_TYPE_SIZE with the
> >> single new hook and appropriate definitions for each target (with a
> >> default definition that uses SFmode for float and DFmode for double and
> >> long double, which would be suitable for many targets).
> >
>
> The originally proposed hook was meant to make the other ports unaffected,
> but I agree that introducing such hook would be more clear.
>
> > 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).
>
> > The most pragmatic solution would be to do
> > s/LONG_DOUBLE_TYPE_SIZE/LONG_DOUBLE_TYPE_MODE/
>
> Yeah, this beats my proposed hook (assuming the default is VOIDmode too).
>
> So it seems we have three alternatives here:
>   1) s/LONG_DOUBLE_TYPE_SIZE/LONG_DOUBLE_TYPE_MODE/
>   2) mode_for_floating_type
>   3) mode_for_abi_type
>
> Since 1) would make long double type special (different from the other types
> having _TYPE_SIZE), personally I'm inclined to 3): implement 2) first, get
> this patch series landed, extend to all.
>
> Do you have any preference?

Maybe do 3) but have the default hook implementation look at
*_TYPE_SIZE when the target doesn't implement the hook?  That would
force you to transition rs6000 away from *_TYPE_SIZE completely
but this would also prove the design.

Btw, for .c.mode_for_abi_type I'd exclude ADA_LONG_TYPE_SIZE.

Joseph, do you agree with this?  I'd not touch the target macros like
PTRDIFF_TYPE (those evaluating to a string) at this point though
they could be handled with a common target hook as well (not sure
if we'd want to have a unified hook for both?).

Thanks,
Richard.

>
> BR,
> Kewen


Re: [PATCH 1/4] rs6000: Make all 128 bit scalar FP modes have 128 bit precision [PR112993]

2024-05-09 Thread Richard Biener
On Thu, May 9, 2024 at 9:12 PM Joseph Myers  wrote:
>
> On Wed, 8 May 2024, Kewen.Lin wrote:
>
> > to widen IFmode to TFmode.  To make build_common_tree_nodes
> > be able to find the correct mode for long double type node,
> > it introduces one hook mode_for_longdouble to offer target
> > a way to specify the mode used for long double type node.
>
> I don't really like layering a hook on top of the old target macro as a
> way to address a deficiency in the design of that target macro (floating
> types should have their mode, not a poorly defined precision value,
> specified directly by the target).

Seconded.

> A better hook design might be something like mode_for_floating_type (enum
> tree_index), where the argument is TI_FLOAT_TYPE, TI_DOUBLE_TYPE or
> TI_LONG_DOUBLE_TYPE, replacing all definitions and uses of
> FLOAT_TYPE_SIZE, DOUBLE_TYPE_SIZE and LONG_DOUBLE_TYPE_SIZE with the
> single new hook and appropriate definitions for each target (with a
> default definition that uses SFmode for float and DFmode for double and
> long double, which would be suitable for many targets).

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.

The most pragmatic solution would be to do
s/LONG_DOUBLE_TYPE_SIZE/LONG_DOUBLE_TYPE_MODE/

Richard.

> --
> Joseph S. Myers
> josmy...@redhat.com
>


Re: [PATCH 1/4] rs6000: Make all 128 bit scalar FP modes have 128 bit precision [PR112993]

2024-05-09 Thread Joseph Myers
On Wed, 8 May 2024, Kewen.Lin wrote:

> to widen IFmode to TFmode.  To make build_common_tree_nodes
> be able to find the correct mode for long double type node,
> it introduces one hook mode_for_longdouble to offer target
> a way to specify the mode used for long double type node.

I don't really like layering a hook on top of the old target macro as a 
way to address a deficiency in the design of that target macro (floating 
types should have their mode, not a poorly defined precision value, 
specified directly by the target).

A better hook design might be something like mode_for_floating_type (enum 
tree_index), where the argument is TI_FLOAT_TYPE, TI_DOUBLE_TYPE or 
TI_LONG_DOUBLE_TYPE, replacing all definitions and uses of 
FLOAT_TYPE_SIZE, DOUBLE_TYPE_SIZE and LONG_DOUBLE_TYPE_SIZE with the 
single new hook and appropriate definitions for each target (with a 
default definition that uses SFmode for float and DFmode for double and 
long double, which would be suitable for many targets).

-- 
Joseph S. Myers
josmy...@redhat.com