On Mon, Mar 07, 2022 at 03:37:18PM -0600, Segher Boessenkool wrote: > Hi! > > On Sat, Mar 05, 2022 at 09:21:51AM +0100, Jakub Jelinek wrote: > > As mentioned in the PR, right now on powerpc* __SIZEOF_{FLOAT,IBM}128__ > > macros are predefined unconditionally, because {ieee,ibm}128_float_type_node > > is always non-NULL, doesn't reflect whether __ieee128 or __ibm128 are > > actually supported or not. > > > > The following patch: > > 1) makes those {ieee,ibm}128_float_type_node trees NULL if we don't > > really support them instead of equal to long_double_type_node > > 2) adjusts the builtins code to use > > ibm128_float_type_node ? ibm128_float_type_node : long_double_type_node > > for the 2 builtins, so that we don't ICE during builtins creation > > if ibm128_float_type_node is NULL (using error_mark_node instead of > > long_double_type_node sounds more risky to me) > > I feel the opposite way: (potentially) using the wrong thing is just a > ticking time bomb, never "safer". > > > 3) in rs6000_type_string will not match NULL as __ibm128, and adds > > a __ieee128 case > > 4) removes the clearly unused ptr_{ieee,ibm}128_float_type_node trees; > > if something needs them in the future, they can be easily added back, > > but wasting GTY just in case... > > 5) actually syncs __SIZEOF_FLOAT128__ with the __float128 macro being > > defined in addition to __ieee128 being usable > > > > Now, in the PR Segher said he doesn't like 5), but I think it is better > > to match the reality and get this PR fixed and if we want to rethink > > how __float128 is defined (whether it is a macro, or perhaps another > > builtin type like __ieee128 which could be easily done by > > lang_hooks.types.register_builtin_type (ieee128_float_type_node, > > "__ieee128"); > > lang_hooks.types.register_builtin_type (ieee128_float_type_node, > > "__float128"); > > perhaps under some conditions, rethink if the -mfloat128 option exists > > and what it does etc., it can be done incrementally and the rs6000-c.cc > > hunks in the patch can be easily reverted (appart from the formatting > > fix). > > There needs to be a __SIZEOF_IEEE128__ as well, if you like reality :-) > Sorry I did not pick up on that earlier.
No, no, no. The '__ieee128' keyword was used as a way to define the type but not enable the '__float128' keyword. Then rs6000-c.cc defines __float128 to be __ieee128, similar to defining 'vector' and '__vector' to be __attribute__((altivec(vector__))'. Unfortunately, there is no way to remove a keyword after the creation (or at least there wasn't in the GCC 8 time frame when I wrote the code), and you need to create the types at GCC startup to set up the built-ins. No one should be using '__ieee128'. The official keywords are '__float128' and for C (not C++) '_Float128'. Perhaps in GCC 13 it is time to just remove it and always just define '__float128' instead. -- Michael Meissner, IBM PO Box 98, Ayer, Massachusetts, USA, 01432 email: meiss...@linux.ibm.com