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

[Michael Meissner <meissner at linux dot vnet dot ibm dot com>]

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

Attachment: gcc-power9.patch305b
Description: Text document