On Thu, Nov 16, 2017 at 04:48:18AM -0600, Segher Boessenkool wrote:
> On Wed, Nov 15, 2017 at 04:56:10PM -0500, Michael Meissner wrote:
> > David tells me that the patch to enable float128 built-in functions to work
> > with the -mabi=ieeelongdouble option broke AIX because on AIX, the float128
> > insns are disabled, and they all become CODE_FOR_nothing. The switch
> > statement
> > that was added in rs6000.c to map KFmode built-in functions to TFmode breaks
> > under AIX.
>
> It also breaks on Linux with older binutils (no HAVE_AS_POWER9 defined).
>
> > I changed the code to have a separate table, and the first call, I build the
> > table. If the insn was not generated, it will just be CODE_FOR_nothing, and
> > the KF->TF mode conversion will not be done.
> >
> > I have tested this on a little endian power8 system and there were no
> > regressions. Once David verifies that it builds on AIX, can I check this
> > into
> > the trunk?
>
> I don't like this scheme much (huge table, initialisation at runtime, etc.),
> but okay for trunk, to unbreak things there.
>
> Some comments on the patch:
>
> > + if (first_time)
> > + {
> > + first_time = false;
> > + gcc_assert ((int)CODE_FOR_nothing == 0);
>
> No useless cast please. The whole assert is pretty useless fwiw; just
> take it out?
>
> > + for (i = 0; i < ARRAY_SIZE (map); i++)
> > + map_insn_code[(int)map[i].from] = map[i].to;
> > + }
>
> Space after cast.
>
> Only do this for codes that are *not* CODE_FOR_nothing?
I must admit to not liking the code, and it is overly complicated.
It occurred to me this morning that a much simpler patch is to just #ifdef out
the switch statement if we don't have the proper assembler. I tried this on an
old power7 system using the system assembler (which does not support the ISA
3.0 instructions) and it built fine. I think this will work on AIX. David can
you check this?
I will fire off a build, and if it is successful, can I check this patch
instead of the other patch?
2017-11-15 Michael Meissner <meiss...@linux.vnet.ibm.com>
* config/rs6000/rs6000.c (rs6000_expand_builtin): Do not do the
switch statement mapping KF built-ins to TF built-ins if we don't
have the proper ISA 3.0 assembler support.
--
Michael Meissner, IBM
IBM, M/S 2506R, 550 King Street, Littleton, MA 01460-6245, USA
email: meiss...@linux.vnet.ibm.com, phone: +1 (978) 899-4797
Index: gcc/config/rs6000/rs6000.c
===================================================================
--- gcc/config/rs6000/rs6000.c (revision 254837)
+++ gcc/config/rs6000/rs6000.c (working copy)
@@ -16690,7 +16690,10 @@ rs6000_expand_builtin (tree exp, rtx tar
double (KFmode) or long double is IEEE 128-bit (TFmode). It is simpler if
we only define one variant of the built-in function, and switch the code
when defining it, rather than defining two built-ins and using the
- overload table in rs6000-c.c to switch between the two. */
+ overload table in rs6000-c.c to switch between the two. If we don't have
+ the proper assembler, don't do this switch because CODE_FOR_*kf* and
+ CODE_FOR_*tf* will be CODE_FOR_nothing. */
+#ifdef HAVE_AS_POWER9
if (FLOAT128_IEEE_P (TFmode))
switch (icode)
{
@@ -16711,6 +16714,7 @@ rs6000_expand_builtin (tree exp, rtx tar
case CODE_FOR_xsiexpqpf_kf: icode = CODE_FOR_xsiexpqpf_tf; break;
case CODE_FOR_xststdcqp_kf: icode = CODE_FOR_xststdcqp_tf; break;
}
+#endif
if (TARGET_DEBUG_BUILTIN)
{
