[Bug target/99708] __SIZEOF_FLOAT128__ not defined on powerpc64le-linux

["segher at gcc dot gnu.org" <gcc-bugzilla@gcc.gnu.org>]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=99708

--- Comment #20 from Segher Boessenkool <segher at gcc dot gnu.org> ---
(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?