[PATCH #2], make Float128 built-in functions work with -mabi=ieeelongdouble

[Michael Meissner]

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  <meissner@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: meissner@linux.vnet.ibm.com, phone: +1 (978) 899-4797
-------------- next part --------------
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)
     {