Re: [PATCH 1/4] rs6000: Make all 128 bit scalar FP modes have 128 bit precision [PR112993]
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]
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]
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]
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]
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