[Bug target/99708] __SIZEOF_FLOAT128__ not defined on powerpc64le-linux
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=99708 Michael Meissner changed: What|Removed |Added Status|NEW |RESOLVED Resolution|--- |FIXED --- Comment #39 from Michael Meissner --- Changes were checked in on March 10th, 2022 by Jakub Jelinek to add these defines.
[Bug target/99708] __SIZEOF_FLOAT128__ not defined on powerpc64le-linux
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=99708 Jonathan Wakely changed: What|Removed |Added Target Milestone|--- |12.0
[Bug target/99708] __SIZEOF_FLOAT128__ not defined on powerpc64le-linux
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=99708 --- Comment #38 from Jonathan Wakely --- (In reply to Peter Bergner from comment #36) > Is this fixed now? Yes, I think the macros are defined consistently with the types now. Let's close it.
[Bug target/99708] __SIZEOF_FLOAT128__ not defined on powerpc64le-linux
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=99708 --- Comment #37 from CVS Commits --- The master branch has been updated by Alexandre Oliva : https://gcc.gnu.org/g:86b31d583a3657f11d930ff156c07b2e20ab05eb commit r13-7191-g86b31d583a3657f11d930ff156c07b2e20ab05eb Author: Alexandre Oliva Date: Fri Apr 14 23:53:36 2023 -0300 rs6000: don't expect __ibm128 with 64-bit long double [PR99708] When long double is 64-bit wide, as on vxworks, the rs6000 backend defines neither the __ibm128 type nor the __SIZEOF_IBM128__ macro, but pr99708.c expected both to be always defined. Adjust the test to match the implementation. Co-Authored-By: Kewen Lin for gcc/testsuite/ChangeLog PR target/99708 * gcc.target/powerpc/pr99708.c: Accept lack of __SIZEOF_IBM128__ when long double is 64-bit wide.
[Bug target/99708] __SIZEOF_FLOAT128__ not defined on powerpc64le-linux
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=99708 Peter Bergner changed: What|Removed |Added CC||bergner at gcc dot gnu.org --- Comment #36 from Peter Bergner --- Is this fixed now?
[Bug target/99708] __SIZEOF_FLOAT128__ not defined on powerpc64le-linux
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=99708 --- Comment #35 from CVS Commits --- The releases/gcc-10 branch has been updated by Michael Meissner : https://gcc.gnu.org/g:8437794102e86a1bd5f2257aa95ea76890810a28 commit r10-10493-g8437794102e86a1bd5f2257aa95ea76890810a28 Author: Michael Meissner Date: Fri Mar 11 19:09:20 2022 -0500 Revert __SIZEOF__IBM128__ and __SIZEOF_FLOAT128__ patch. 2022-03-05 Michael Meissner gcc/ PR target/99708 * config/rs6000/rs6000-c.c: Revert 2022-03-05 patch. gcc/testsuite/ PR target/99708 * gcc.target/powerpc/pr99708.c: Revert 2022-03-05 patch.
[Bug target/99708] __SIZEOF_FLOAT128__ not defined on powerpc64le-linux
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=99708 --- Comment #34 from CVS Commits --- The releases/gcc-11 branch has been updated by Michael Meissner : https://gcc.gnu.org/g:6f581f90e3757392a510f11279e2daf5fcfdefa8 commit r11-9649-g6f581f90e3757392a510f11279e2daf5fcfdefa8 Author: Michael Meissner Date: Fri Mar 11 18:41:20 2022 -0500 Revert __SIZEOF__IBM128__ and __SIZEOF_FLOAT128__ patch. 2022-03-05 Michael Meissner gcc/ PR target/99708 * config/rs6000/rs6000-c.c: Revert patch from 2022-03-05. gcc/testsuite/ PR target/99708 * gcc.target/powerpc/pr99708.c: Revert patch from 2022-03-05.
[Bug target/99708] __SIZEOF_FLOAT128__ not defined on powerpc64le-linux
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=99708 --- Comment #33 from Jakub Jelinek --- In https://gcc.gnu.org/pipermail/gcc-patches/2022-March/591521.html Segher said those backports should be reverted.
[Bug target/99708] __SIZEOF_FLOAT128__ not defined on powerpc64le-linux
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=99708 Eric Botcazou changed: What|Removed |Added CC||ebotcazou at gcc dot gnu.org --- Comment #32 from Eric Botcazou --- We have regressions for VxWorks on the gcc-10 and gcc-11 branches because of the first backports, do you plan to backport the fix onto these branches?
[Bug target/99708] __SIZEOF_FLOAT128__ not defined on powerpc64le-linux
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=99708 --- Comment #31 from CVS Commits --- The master branch has been updated by Jakub Jelinek : https://gcc.gnu.org/g:6f8abf2b9ff4f165a61295cdb3525ce1da2a77c6 commit r12-7576-g6f8abf2b9ff4f165a61295cdb3525ce1da2a77c6 Author: Jakub Jelinek Date: Thu Mar 10 10:22:27 2022 +0100 rs6000: Fix up __SIZEOF_{FLOAT,IBM}128__ defines [PR99708] 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. Based on patch review discussions, the following patch: 1) allows __ibm128 to be used in the sources even when !TARGET_FLOAT128_TYPE, as long as long double is double double 2) ensures ibm128_float_type_node is non-NULL only if __ibm128 is supported 3) ensures ieee128_float_type_node is non-NULL only if __ieee128 is supported (aka when TARGET_FLOAT128_TYPE) 4) predefines __SIZEOF_IBM128__ only when ibm128_float_type_node != NULL 5) newly predefines __SIZEOF_IEEE128__ if ieee128_float_type_node != NULL 6) predefines __SIZEOF_FLOAT128__ whenever ieee128_float_type_node != NULL and __float128 macro is predefined to __ieee128 7) removes ptr_*128_float_type_node which nothing uses 8) in order not to ICE during builtin initialization when ibm128_float_type_node == NULL, uses long_double_type_node as fallback for the __builtin_{,un}pack_ibm128 builtins 9) errors when those builtins are called used when ibm128_float_type_node == NULL (during their expansion) 10) moves the {,un}packif -> {,un}packtf remapping for these builtins in expansion earlier, so that we don't ICE on them if not -mabi=ieeelongdouble 2022-03-10 Jakub Jelinek PR target/99708 * config/rs6000/rs6000.h (enum rs6000_builtin_type_index): Remove RS6000_BTI_ptr_ieee128_float and RS6000_BTI_ptr_ibm128_float. (ptr_ieee128_float_type_node, ptr_ibm128_float_type_node): Remove. * config/rs6000/rs6000-builtin.cc (rs6000_type_string): Return "**NULL**" if type_node is NULL first. Handle ieee128_float_type_node. (rs6000_init_builtins): Don't initialize ptr_ieee128_float_type_node and ptr_ibm128_float_type_node. Set ibm128_float_type_node and ieee128_float_type_node to NULL rather than long_double_type_node if they aren't supported. Do support __ibm128 even if !TARGET_FLOAT128_TYPE when long double is double double. (rs6000_expand_builtin): Error if bif_is_ibm128 and !ibm128_float_type_node. Remap RS6000_BIF_{,UN}PACK_IF to RS6000_BIF_{,UN}PACK_TF much earlier and only use bif_is_ibm128 check for it. * config/rs6000/rs6000-c.cc (rs6000_target_modify_macros): Define __SIZEOF_FLOAT128__ here and only iff __float128 macro is defined. (rs6000_cpu_cpp_builtins): Don't define __SIZEOF_FLOAT128__ here. Define __SIZEOF_IBM128__=16 if ieee128_float_type_node is non-NULL. Formatting fix. * config/rs6000/rs6000-gen-builtins.cc: Document ibm128 attribute. (struct attrinfo): Add isibm128 member. (TYPE_MAP_SIZE): Remove. (type_map): Use [] instead of [TYPE_MAP_SIZE]. For "if" use ibm128_float_type_node only if it is non-NULL, otherwise fall back to long_double_type_node. Remove "pif" entry. (parse_bif_attrs): Handle ibm128 attribute and print it for debugging. (write_decls): Output bif_ibm128_bit and bif_is_ibm128. (write_type_node): Use sizeof type_map / sizeof type_map[0] instead of TYPE_MAP_SIZE. (write_bif_static_init): Handle isibm128. * config/rs6000/rs6000-builtins.def: Document ibm128 attribute. (__builtin_pack_ibm128, __builtin_unpack_ibm128): Add ibm128 attribute. * gcc.dg/pr99708.c: New test. * gcc.target/powerpc/pr99708-2.c: New test. * gcc.target/powerpc/convert-fp-128.c (mode_kf): Define only if __FLOAT128_TYPE__ is defined.
[Bug target/99708] __SIZEOF_FLOAT128__ not defined on powerpc64le-linux
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=99708 --- Comment #30 from Segher Boessenkool --- There should be a __SIZEOF_IEEE128__ as well, of course.
[Bug target/99708] __SIZEOF_FLOAT128__ not defined on powerpc64le-linux
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=99708 --- Comment #29 from CVS Commits --- The releases/gcc-10 branch has been updated by Michael Meissner : https://gcc.gnu.org/g:641b407763ecfee5d4ac86d8ffe9eb1eeea5fd10 commit r10-10486-g641b407763ecfee5d4ac86d8ffe9eb1eeea5fd10 Author: Michael Meissner Date: Sat Mar 5 20:11:38 2022 -0500 Define __SIZEOF_FLOAT128__ and __SIZEOF_IBM128__. Define the sizes of the PowerPC specific types __float128 and __ibm128 if those types are enabled. This patch will define __SIZEOF_IBM128__ and __SIZEOF_FLOAT128__ if their respective types are created in the compiler. Currently, this means both of these will be defined if float128 support is enabled. But at some point in the future, __ibm128 could be enabled without enabling float128 support and __SIZEOF_IBM128__ would be defined. 2022-03-05 Michael Meissner gcc/ PR target/99708 * config/rs6000/rs6000-c.c (rs6000_cpu_cpp_builtins): Define __SIZEOF_IBM128__ if the IBM 128-bit long double type is created. Define __SIZEOF_FLOAT128__ if the IEEE 128-bit floating point type is created. Backport change to master branch on 2022-02-17. gcc/testsuite/ PR target/99708 * gcc.target/powerpc/pr99708.c: New test. Backport change to master branch on 2022-02-17.
[Bug target/99708] __SIZEOF_FLOAT128__ not defined on powerpc64le-linux
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=99708 --- Comment #28 from CVS Commits --- The releases/gcc-11 branch has been updated by Michael Meissner : https://gcc.gnu.org/g:fa944e8660e158655beda58e12b69fd18dd49108 commit r11-9639-gfa944e8660e158655beda58e12b69fd18dd49108 Author: Michael Meissner Date: Sat Mar 5 11:22:15 2022 -0500 Define __SIZEOF_FLOAT128__ and __SIZEOF_IBM128__. Define the sizes of the PowerPC specific types __float128 and __ibm128 if those types are enabled. This patch will define __SIZEOF_IBM128__ and __SIZEOF_FLOAT128__ if their respective types are created in the compiler. Currently, this means both of these will be defined if float128 support is enabled. But at some point in the future, __ibm128 could be enabled without enabling float128 support and __SIZEOF_IBM128__ would be defined. 2022-03-05 Michael Meissner gcc/ PR target/99708 * config/rs6000/rs6000-c.c (rs6000_cpu_cpp_builtins): Define __SIZEOF_IBM128__ if the IBM 128-bit long double type is created. Define __SIZEOF_FLOAT128__ if the IEEE 128-bit floating point type is created. Backport change made to the master branch on 2022-02-17. gcc/testsuite/ PR target/99708 * gcc.target/powerpc/pr99708.c: New test. Backport change made to the master branch on 2022-02-17.
[Bug target/99708] __SIZEOF_FLOAT128__ not defined on powerpc64le-linux
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=99708 --- Comment #27 from Segher Boessenkool --- (In reply to Jakub Jelinek from comment #26) > (In reply to Segher Boessenkool from comment #25) > > It is defined to __ieee128 whenever that exists, and not defined otherwise? > > Yes, the logic and control flow are byzantine. > > No, far from it. > E.g. on linux -mlong-double-128 -mabi=ieeelongdouble -mno-float128 > means that __ieee128 works, long double works too and are the same, but > __float128 doesn't. > Even worse (and the reason why I've moved it to a different function) is that > one can do > #pragma GCC target ("no-float128") > or > #pragma GCC target ("float128") > in the middle of the source. Both of those qualify as bugs themselves, in my book. Those behaviours are not useful, and make life extremely hard for both users and GCC implementors.
[Bug target/99708] __SIZEOF_FLOAT128__ not defined on powerpc64le-linux
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=99708 --- Comment #26 from Jakub Jelinek --- (In reply to Segher Boessenkool from comment #25) > It is defined to __ieee128 whenever that exists, and not defined otherwise? > Yes, the logic and control flow are byzantine. No, far from it. E.g. on linux -mlong-double-128 -mabi=ieeelongdouble -mno-float128 means that __ieee128 works, long double works too and are the same, but __float128 doesn't. Even worse (and the reason why I've moved it to a different function) is that one can do #pragma GCC target ("no-float128") or #pragma GCC target ("float128") in the middle of the source.
[Bug target/99708] __SIZEOF_FLOAT128__ not defined on powerpc64le-linux
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=99708 --- Comment #25 from Segher Boessenkool --- It is defined to __ieee128 whenever that exists, and not defined otherwise? Yes, the logic and control flow are byzantine.
[Bug target/99708] __SIZEOF_FLOAT128__ not defined on powerpc64le-linux
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=99708 --- Comment #24 from Jakub Jelinek --- (In reply to Segher Boessenkool from comment #22) > In rs6000_type_string, please just handle the error !type_node case first, > so you don't have to consider it in all other cases separately. Ok, will do. > Do you need to change the __SIZEOF_FLOAT128__ code at all now? I don't > see it? Yes, because the macro is from name about whether __float128 can be used and what it's size. On powerpc*, __float128 is a macro defined to __ieee128 under some conditions, so the condition to define __SIZEOF_FLOAT128__ should be that the macro is defined and __ieee128 can be used.
[Bug target/99708] __SIZEOF_FLOAT128__ not defined on powerpc64le-linux
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=99708 --- Comment #23 from Segher Boessenkool --- Oh, and looks great, thank you!
[Bug target/99708] __SIZEOF_FLOAT128__ not defined on powerpc64le-linux
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=99708 --- Comment #22 from Segher Boessenkool --- In rs6000_type_string, please just handle the error !type_node case first, so you don't have to consider it in all other cases separately. Do you need to change the __SIZEOF_FLOAT128__ code at all now? I don't see it?
[Bug target/99708] __SIZEOF_FLOAT128__ not defined on powerpc64le-linux
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=99708 --- Comment #21 from Jakub Jelinek --- Created attachment 52566 --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=52566=edit gcc12-pr99708.patch Updated patch I'm going to test.
[Bug target/99708] __SIZEOF_FLOAT128__ not defined on powerpc64le-linux
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=99708 --- Comment #20 from Segher Boessenkool --- (In reply to Jakub Jelinek from comment #19) > I'd guess that else ieee128_float_type_node = ibm128_float_type_node = > long_double_type_node; > is there so that we don't ICE during the builtins creation Probably. But is is completely wrong, and causes pretty bad problems for a tiny short-term benefit, as this whole affair shows. > Looking at other uses: > rs6000_type_string does: > else if (type_node == ibm128_float_type_node) > return "__ibm128"; > could add type_node && to it. Yes. > And it also makes me wonder why there is no ieee128_float_type_node case. Good question :-( > Rest of rs6000-builtin.cc is just the setup and could live without > else ieee128_float_type_node = ibm128_float_type_node = > long_double_type_node; Or = 0 even. Yes. > Also, why do we have ptr_*_float_type_node at all when nothing uses those? Maybe the old builtin stuff used that? Dunno, just guessing. > rs6000.cc will be fine even with *128_float_type_node NULL, > long_double_type_node is presumably always non-NULL. I sure hope so, it is a required type after all :-) > rs6000-c.cc is what this PR talks about, so actually needs those to be NULL > if not supported > (but, we still want to move the __SIZEOF_FLOAT128__ handling next to > __float128 macro IMNSHO, > if we add also __SIZEOF_IEEE128__ it could stay where __SIZEOF_FLOAT128__ is > defined now). I'm not sure what that would look like? I certainly do agree that the code is a bit of a mess and could use some improvement. > And finally the generated rs6000-builtins.cc, it does: > tree df_ftype_if_ci > = build_function_type_list (double_type_node, > ibm128_float_type_node, > integer_type_node, > NULL_TREE); > tree if_ftype_df_df > = build_function_type_list (ibm128_float_type_node, > double_type_node, > double_type_node, > NULL_TREE); > rs6000_builtin_info[RS6000_BIF_PACK_IF].fntype > = if_ftype_df_df; > rs6000_builtin_decls[(int)RS6000_BIF_PACK_IF] = t > = add_builtin_function ("__builtin_pack_ibm128", > if_ftype_df_df, > (int)RS6000_BIF_PACK_IF, BUILT_IN_MD, > NULL, NULL_TREE); > TREE_READONLY (t) = 1; > TREE_NOTHROW (t) = 1; > rs6000_builtin_info[RS6000_BIF_UNPACK_IF].fntype > = df_ftype_if_ci; > rs6000_builtin_decls[(int)RS6000_BIF_UNPACK_IF] = t > = add_builtin_function ("__builtin_unpack_ibm128", > df_ftype_if_ci, > (int)RS6000_BIF_UNPACK_IF, BUILT_IN_MD, > NULL, NULL_TREE); > TREE_READONLY (t) = 1; > TREE_NOTHROW (t) = 1; > Unfortunately it is a generated file. Dunno what is best for that, > not registering the builtins at all if ibm128_float_type_node is NULL, > or keep doing what it used to, register those with some other type. Either choice will not change the semantics of any valid program, so that is good. Of course it isn't very friendly to ICE on incorrect programs ;-) Maybe we can register the builtins with zero type just like we do already, or maybe a void type or such? Is there an explicit "error" type we could use?
[Bug target/99708] __SIZEOF_FLOAT128__ not defined on powerpc64le-linux
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=99708 --- Comment #19 from Jakub Jelinek --- builddir/gcc/rs6000-builtins.cc: ibm128_float_type_node, builddir/gcc/rs6000-builtins.cc:= build_function_type_list (ibm128_float_type_node, rs6000-builtin.cc: else if (type_node == ibm128_float_type_node) rs6000-builtin.cc: ibm128_float_type_node = long_double_type_node; rs6000-builtin.cc:ibm128_float_type_node = make_node (REAL_TYPE); rs6000-builtin.cc:TYPE_PRECISION (ibm128_float_type_node) = 128; rs6000-builtin.cc:SET_TYPE_MODE (ibm128_float_type_node, IFmode); rs6000-builtin.cc:layout_type (ibm128_float_type_node); rs6000-builtin.cc: t = build_qualified_type (ibm128_float_type_node, TYPE_QUAL_CONST); rs6000-builtin.cc: ptr_ibm128_float_type_node = build_pointer_type (t); rs6000-builtin.cc: lang_hooks.types.register_builtin_type (ibm128_float_type_node, rs6000-builtin.cc: ieee128_float_type_node = long_double_type_node; rs6000-builtin.cc: ieee128_float_type_node = float128_type_node; rs6000-builtin.cc: t = build_qualified_type (ieee128_float_type_node, TYPE_QUAL_CONST); rs6000-builtin.cc: ptr_ieee128_float_type_node = build_pointer_type (t); rs6000-builtin.cc: lang_hooks.types.register_builtin_type (ieee128_float_type_node, rs6000-builtin.cc:ieee128_float_type_node = ibm128_float_type_node = long_double_type_node; rs6000.cc: && ieee128_float_type_node == long_double_type_node) rs6000.cc:&& ibm128_float_type_node == long_double_type_node)) rs6000-c.cc: if (ibm128_float_type_node) rs6000-c.cc: if (ieee128_float_type_node) rs6000.h:#define ieee128_float_type_node (rs6000_builtin_types[RS6000_BTI_ieee128_float]) rs6000.h:#define ibm128_float_type_node (rs6000_builtin_types[RS6000_BTI_ibm128_float]) rs6000.h:#define ptr_ieee128_float_type_node (rs6000_builtin_types[RS6000_BTI_ptr_ieee128_float]) rs6000.h:#define ptr_ibm128_float_type_node (rs6000_builtin_types[RS6000_BTI_ptr_ibm128_float]) I'd guess that else ieee128_float_type_node = ibm128_float_type_node = long_double_type_node; is there so that we don't ICE during the builtins creation (we need just ibm128_float_type_node for it though). Looking at other uses: rs6000_type_string does: else if (type_node == ibm128_float_type_node) return "__ibm128"; could add type_node && to it. And it also makes me wonder why there is no ieee128_float_type_node case. Rest of rs6000-builtin.cc is just the setup and could live without else ieee128_float_type_node = ibm128_float_type_node = long_double_type_node; Also, why do we have ptr_*_float_type_node at all when nothing uses those? rs6000.cc will be fine even with *128_float_type_node NULL, long_double_type_node is presumably always non-NULL. rs6000-c.cc is what this PR talks about, so actually needs those to be NULL if not supported (but, we still want to move the __SIZEOF_FLOAT128__ handling next to __float128 macro IMNSHO, if we add also __SIZEOF_IEEE128__ it could stay where __SIZEOF_FLOAT128__ is defined now). And finally the generated rs6000-builtins.cc, it does: tree df_ftype_if_ci = build_function_type_list (double_type_node, ibm128_float_type_node, integer_type_node, NULL_TREE); tree if_ftype_df_df = build_function_type_list (ibm128_float_type_node, double_type_node, double_type_node, NULL_TREE); rs6000_builtin_info[RS6000_BIF_PACK_IF].fntype = if_ftype_df_df; rs6000_builtin_decls[(int)RS6000_BIF_PACK_IF] = t = add_builtin_function ("__builtin_pack_ibm128", if_ftype_df_df, (int)RS6000_BIF_PACK_IF, BUILT_IN_MD, NULL, NULL_TREE); TREE_READONLY (t) = 1; TREE_NOTHROW (t) = 1; rs6000_builtin_info[RS6000_BIF_UNPACK_IF].fntype = df_ftype_if_ci; rs6000_builtin_decls[(int)RS6000_BIF_UNPACK_IF] = t = add_builtin_function ("__builtin_unpack_ibm128", df_ftype_if_ci, (int)RS6000_BIF_UNPACK_IF, BUILT_IN_MD, NULL, NULL_TREE); TREE_READONLY (t) = 1; TREE_NOTHROW (t) = 1; Unfortunately it is a generated file. Dunno what is best for that, not registering the builtins at all if ibm128_float_type_node is NULL, or keep doing what it used to, register those with some other type.
[Bug target/99708] __SIZEOF_FLOAT128__ not defined on powerpc64le-linux
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=99708 --- Comment #18 from Segher Boessenkool --- Ah, I didn't see the else ieee128_float_type_node = ibm128_float_type_node = long_double_type_node; which looks completely garbage. It long double is just DP float, we certainly do not want either __ibm128 or __ieee128 to be the same! Mike?
[Bug target/99708] __SIZEOF_FLOAT128__ not defined on powerpc64le-linux
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=99708 --- Comment #17 from Segher Boessenkool --- (In reply to Jakub Jelinek from comment #13) > I see > Doesn't this mean that ieee128_float_type_node and ibm128_float_type_node is > always non-NULL? No. All of that code is inside if (TARGET_FLOAT128_TYPE) so none of that is ever run otherwise. The basic types we have are __ibm128 and __ieee128, all the rest is a macro maze.
[Bug target/99708] __SIZEOF_FLOAT128__ not defined on powerpc64le-linux
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=99708 --- Comment #16 from Jakub Jelinek --- So what about: --- gcc/config/rs6000/rs6000-c.cc.jj2022-02-17 10:24:16.756113275 +0100 +++ gcc/config/rs6000/rs6000-c.cc 2022-03-03 19:06:25.771981905 +0100 @@ -584,6 +584,10 @@ rs6000_target_modify_macros (bool define rs6000_define_or_undefine_macro (true, "__float128=__ieee128"); else rs6000_define_or_undefine_macro (false, "__float128"); + if (ibm128_float_type_node != ieee128_float_type_node && define_p) + rs6000_define_or_undefine_macro (true, "__SIZEOF_FLOAT128__=16"); + else + rs6000_define_or_undefine_macro (false, "__SIZEOF_FLOAT128__"); } /* OPTION_MASK_FLOAT128_HARDWARE can be turned on if -mcpu=power9 is used or via the target attribute/pragma. */ @@ -623,11 +627,9 @@ rs6000_cpu_cpp_builtins (cpp_reader *pfi if (TARGET_FRSQRTES) builtin_define ("__RSQRTEF__"); if (TARGET_FLOAT128_TYPE) - builtin_define ("__FLOAT128_TYPE__"); - if (ibm128_float_type_node) +builtin_define ("__FLOAT128_TYPE__"); + if (ibm128_float_type_node != ieee128_float_type_node) builtin_define ("__SIZEOF_IBM128__=16"); - if (ieee128_float_type_node) -builtin_define ("__SIZEOF_FLOAT128__=16"); #ifdef TARGET_LIBC_PROVIDES_HWCAP_IN_TCB builtin_define ("__BUILTIN_CPU_SUPPORTS__"); #endif ?
[Bug target/99708] __SIZEOF_FLOAT128__ not defined on powerpc64le-linux
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=99708 --- Comment #15 from Jakub Jelinek --- Perhaps another possible test could be ieee128_float_type_node != ibm128_float_type_node because whenever those two are different, we know __ieee128 and __ibm128 are supported (but still need to verify whether __float128 macro is defined).
[Bug target/99708] __SIZEOF_FLOAT128__ not defined on powerpc64le-linux
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=99708 --- Comment #14 from Jakub Jelinek --- Unfortunately, checking TYPE_NAME won't work either. Because for the ibm128_float_type_node = long_double_type_node; and ieee128_float_type_node = long_double_type_node; cases, lang_hooks.types.register_builtin_type will not change TYPE_NAME on those which remains "long double". As one can't easily name-lookup those __ieee128 and __ibm128 identifiers back, I think TARGET_FLOAT128_TYPE is the macro that controls it. The __SIZEOF_*__ macros probably should be defined/undefined in rs6000_target_modify_macros based on that and for __SIZEOF_FLOAT128__ also based on (flags & OPTION_MASK_FLOAT128_KEYWORD) != 0.
[Bug target/99708] __SIZEOF_FLOAT128__ not defined on powerpc64le-linux
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=99708 --- Comment #13 from Jakub Jelinek --- I see if (TARGET_FLOAT128_TYPE) { if (!TARGET_IEEEQUAD && TARGET_LONG_DOUBLE_128) ibm128_float_type_node = long_double_type_node; else { ibm128_float_type_node = make_node (REAL_TYPE); TYPE_PRECISION (ibm128_float_type_node) = 128; SET_TYPE_MODE (ibm128_float_type_node, IFmode); layout_type (ibm128_float_type_node); } t = build_qualified_type (ibm128_float_type_node, TYPE_QUAL_CONST); ptr_ibm128_float_type_node = build_pointer_type (t); lang_hooks.types.register_builtin_type (ibm128_float_type_node, "__ibm128"); if (TARGET_IEEEQUAD && TARGET_LONG_DOUBLE_128) ieee128_float_type_node = long_double_type_node; else ieee128_float_type_node = float128_type_node; t = build_qualified_type (ieee128_float_type_node, TYPE_QUAL_CONST); ptr_ieee128_float_type_node = build_pointer_type (t); lang_hooks.types.register_builtin_type (ieee128_float_type_node, "__ieee128"); } else ieee128_float_type_node = ibm128_float_type_node = long_double_type_node; Doesn't this mean that ieee128_float_type_node and ibm128_float_type_node is always non-NULL? So, maybe we shouldn't test whether those are non-NULL, but whether the name of say ieee128_float_type_node is __ieee128 and similarly if ibm128_float_type_node's name is __ibm128? Though, __SIZEOF_FLOAT128__ macro talks about __float128 which is on ppc64 a macro, so probably it needs to be that plus whether __float128 is defined to __ieee128.
[Bug target/99708] __SIZEOF_FLOAT128__ not defined on powerpc64le-linux
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=99708 --- Comment #12 from Segher Boessenkool --- (In reply to Jonathan Wakely from comment #10) > Maybe we could do this instead: > > --- a/gcc/config/rs6000/rs6000-c.cc > +++ b/gcc/config/rs6000/rs6000-c.cc > @@ -623,11 +623,13 @@ rs6000_cpu_cpp_builtins (cpp_reader *pfile) >if (TARGET_FRSQRTES) > builtin_define ("__RSQRTEF__"); >if (TARGET_FLOAT128_TYPE) > +{ >builtin_define ("__FLOAT128_TYPE__"); > + if (ieee128_float_type_node) > + builtin_define ("__SIZEOF_FLOAT128__=16"); > +} >if (ibm128_float_type_node) > builtin_define ("__SIZEOF_IBM128__=16"); > - if (ieee128_float_type_node) > -builtin_define ("__SIZEOF_FLOAT128__=16"); > #ifdef TARGET_LIBC_PROVIDES_HWCAP_IN_TCB >builtin_define ("__BUILTIN_CPU_SUPPORTS__"); > #endif But why is ieee128_float_type_node not zero then? That doesn't make much sense, so this would just be hiding problems. > It would be nice to add a test to the testsuite along the lines of: > > /* { dg-do compile } */ > #ifdef __SIZEOF_FLOAT128__ > __float128 f; > #endif Sure, if we want this to be a generic macro, that is an excellent idea. Also for the other types?
[Bug target/99708] __SIZEOF_FLOAT128__ not defined on powerpc64le-linux
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=99708 Segher Boessenkool changed: What|Removed |Added Ever confirmed|0 |1 Last reconfirmed||2022-03-03 Status|UNCONFIRMED |NEW --- Comment #11 from Segher Boessenkool --- (In reply to Jonathan Wakely from comment #8) > But for power7 ppc64 (BE) linux (e.g. gcc110 in the cfarm): > > ./gcc/cc1plus -E -dM /dev/null -quiet | fgrep FLOAT128 > #define __SIZEOF_FLOAT128__ 16 It isn't clear what cpu your compiler defaults to, making this not such a great example. -mcpu=power7 *does* support __float128, after all. But indeed -mcpu=970 defines __SIZEOF_FLOAT128__ as well, which is wrong. Confirmed.
[Bug target/99708] __SIZEOF_FLOAT128__ not defined on powerpc64le-linux
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=99708 --- Comment #10 from Jonathan Wakely --- Maybe we could do this instead: --- a/gcc/config/rs6000/rs6000-c.cc +++ b/gcc/config/rs6000/rs6000-c.cc @@ -623,11 +623,13 @@ rs6000_cpu_cpp_builtins (cpp_reader *pfile) if (TARGET_FRSQRTES) builtin_define ("__RSQRTEF__"); if (TARGET_FLOAT128_TYPE) +{ builtin_define ("__FLOAT128_TYPE__"); + if (ieee128_float_type_node) + builtin_define ("__SIZEOF_FLOAT128__=16"); +} if (ibm128_float_type_node) builtin_define ("__SIZEOF_IBM128__=16"); - if (ieee128_float_type_node) -builtin_define ("__SIZEOF_FLOAT128__=16"); #ifdef TARGET_LIBC_PROVIDES_HWCAP_IN_TCB builtin_define ("__BUILTIN_CPU_SUPPORTS__"); #endif It would be nice to add a test to the testsuite along the lines of: /* { dg-do compile } */ #ifdef __SIZEOF_FLOAT128__ __float128 f; #endif
[Bug target/99708] __SIZEOF_FLOAT128__ not defined on powerpc64le-linux
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=99708 --- Comment #9 from Jonathan Wakely --- It looks like r12-7271-g687e57d7ac741d added it: --- a/gcc/config/rs6000/rs6000-c.cc +++ b/gcc/config/rs6000/rs6000-c.cc @@ -623,7 +623,11 @@ rs6000_cpu_cpp_builtins (cpp_reader *pfile) if (TARGET_FRSQRTES) builtin_define ("__RSQRTEF__"); if (TARGET_FLOAT128_TYPE) -builtin_define ("__FLOAT128_TYPE__"); + builtin_define ("__FLOAT128_TYPE__"); + if (ibm128_float_type_node) +builtin_define ("__SIZEOF_IBM128__=16"); + if (ieee128_float_type_node) +builtin_define ("__SIZEOF_FLOAT128__=16"); #ifdef TARGET_LIBC_PROVIDES_HWCAP_IN_TCB builtin_define ("__BUILTIN_CPU_SUPPORTS__"); #endif
[Bug target/99708] __SIZEOF_FLOAT128__ not defined on powerpc64le-linux
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=99708 --- Comment #8 from Jonathan Wakely --- It looks like trunk now defines __SIZEOF_FLOAT128__ on powerpc-ibm-aix* and powerpc64*-*-linux-gnu, but it seems to be defined unconditionally, even if the __float128 type *isn't* available! On power8-aix (e.g. gcc119 in the cfarm): ./gcc/cc1plus -E -dM /dev/null -quiet -maix64 | fgrep FLOAT128#define __SIZEOF_FLOAT128__ 16 ./gcc/cc1plus - -quiet <<< '__float128 f;' :1:1: error: '__float128' does not name a type; did you mean '__int128'? You need to add -mfloat128 -mvsx to be able to use the type. On power8 ppc64le linux (e.g. gcc112 in the cfarm): ./gcc/cc1plus -E -dM /dev/null -quiet | fgrep FLOAT128 #define __FLOAT128__ 1 #define __SIZEOF_FLOAT128__ 16 #define __FLOAT128_TYPE__ 1 That's fine, the type is usable. But for power7 ppc64 (BE) linux (e.g. gcc110 in the cfarm): ./gcc/cc1plus -E -dM /dev/null -quiet | fgrep FLOAT128 #define __SIZEOF_FLOAT128__ 16 That means __SIZEOF_FLOAT128__ still can't be used to detect whether a type called "__float128" is supported for the current target. On power it looks like we can use __FLOAT128__ for that purpose, but other targets such as x86 don't define that one, only __SIZEOF_FLOAT128__. So you have to use a different macro to detect __float128 depending on the target.
[Bug target/99708] __SIZEOF_FLOAT128__ not defined on powerpc64le-linux
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=99708 --- Comment #7 from Jonathan Wakely --- (In reply to Segher Boessenkool from comment #6) > Yes. And it does not mean the type exist (or is usable), either. Example? > Do we? The types should always exist! Please tell that to our IBM colleagues working on clang. __float128 is not supported for anything less than -mcpu=power9 which means that clang cannot be ABI compatible with GCC when using libstdc++ headers on power8. The current solution is to disable all the work I've done for the IEEE128 transition when compiled with clang. The ABI incompatibility will remain until Clang supports those types properly *and* provides macros to check for the types being supported. It would be nice if GCC and Clang agreed on those macros. > Other targets do not have __ieee128 or __ibm128. But they have other target-specific types and they define __SIZEOF_xxx__ for those types, e.g. __SIZEOF_FLOAT128__. There's a reason I used "xxx" because I'm talking about "non-standard types that aren't available on all targets", where __ieee128 and __ibm128 are examples for powerpc (and are the outliers currently when compared to other targets).
[Bug target/99708] __SIZEOF_FLOAT128__ not defined on powerpc64le-linux
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=99708 --- Comment #6 from Segher Boessenkool --- (In reply to Jonathan Wakely from comment #5) > (In reply to Segher Boessenkool from comment #3) > > In an ideal world the user can just assume those types exist always. > Arguably a __SIZEOF_xxx__ macro isn't a very sensible macro for types where > the type has a guaranteed size, Yes. And it does not mean the type exist (or is usable), either. > but we need *something* that says the type > exists. Do we? The types should always exist! > Since all other targets already use __SIZEOF_xxx__ to say that the > type exists, it would be consistent and helpful for powerpc to do the same. Other targets do not have __ieee128 or __ibm128.
[Bug target/99708] __SIZEOF_FLOAT128__ not defined on powerpc64le-linux
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=99708 --- Comment #5 from Jonathan Wakely --- (In reply to Segher Boessenkool from comment #3) > In an ideal world the user can just assume those types exist always. In a > less ideal world, use autoconf? You have to anyway, if you want to support > older compilers at all. Libstdc++ headers cannot use autoconf to check for features in a compiler that users run on their own machine. Libstdc++ needs to know whether types such as __float128 and __ibm128 are defined **by the compiler that happens to be including the headers**. That might be GCC (in which case we have complete knowledge about what types it defines) or it could be an arbitrary version of clang. For the latter case we cannot run autoconf to probe the user's clang version because libstdc++ has already been installed. We need clang to expose macros that say which types are defined, and obviously it would be preferable for gcc and clang to agree on which macros are used for which types. Arguably a __SIZEOF_xxx__ macro isn't a very sensible macro for types where the type has a guaranteed size, but we need *something* that says the type exists. Since all other targets already use __SIZEOF_xxx__ to say that the type exists, it would be consistent and helpful for powerpc to do the same.
[Bug target/99708] __SIZEOF_FLOAT128__ not defined on powerpc64le-linux
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=99708 --- Comment #4 from Jakub Jelinek --- __SIZEOF_FLOAT128__ is predefined on all targets that have the __float128 type, except for rs6000: grep '"__float128"' */* i386/i386-builtins.c: lang_hooks.types.register_builtin_type (float128_type_node, "__float128"); ia64/ia64.c: "__float128"); ia64/ia64.c: "__float128"); pa/pa.c: "__float128"); rs6000/rs6000-c.c: rs6000_define_or_undefine_macro (false, "__float128"); grep SIZEOF_FLOAT128 */* i386/i386-c.c: cpp_define (parse_in, "__SIZEOF_FLOAT128__=16"); ia64/ia64.h:builtin_define("__SIZEOF_FLOAT128__=16");\ pa/pa.h: builtin_define("__SIZEOF_FLOAT128__=16");\ See also PR56540
[Bug target/99708] __SIZEOF_FLOAT128__ not defined on powerpc64le-linux
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=99708 --- Comment #3 from Segher Boessenkool --- The only such __SIZEOF_* macro that is not about a standards-required type is for int128. Not the best example ;-) There are not predefines for __SIZEOF_FLOAT128__ etc. either. In an ideal world the user can just assume those types exist always. In a less ideal world, use autoconf? You have to anyway, if you want to support older compilers at all.
[Bug target/99708] __SIZEOF_FLOAT128__ not defined on powerpc64le-linux
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=99708 --- Comment #2 from Jakub Jelinek --- The __SIZEOF_*__ macros are widely used to detect both if a type can be used and what sizeof (the_type) is when it needs to be checked in preprocessor conditionals, including hundreds of times in GCC testsuite. It is something supported by multiple compilers (GCC, clang, ICC at least). Having to remember magic macros which are different on each arch and in each compiler is much more complicated. The topic this came up on is libstdc++ headers which in ext/numeric_traits.h wants to provide ppc64le compatibility and right now assumes that if __LONG_DOUBLE_IEEE128__ is defined then __ibm128 type can be used and __LONG_DOUBLE_IBM128__ is defined then __ieee128 type can be used. Apparently that is not the case at least for clang which didn't have __ieee128 support until very recently and __float128 support needs a non-default option passed to the compiler. Which was why I suggested to guard those by # elif defined __LONG_DOUBLE_IBM128__ && __SIZEOF_FLOAT128__ == 16 or something similar, but it doesn't work for GCC.
[Bug target/99708] __SIZEOF_FLOAT128__ not defined on powerpc64le-linux
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=99708 --- Comment #1 from Segher Boessenkool --- Yes, the __SIZEOF_* macros do not say whether some type can be used. This is true for all targets! What would it be useful for to define these macros? They all are equivalent to #define SIXTEEN 16 :-)