[PATCH], PowerPC: Use <math>f128 for long double built-ins if we have changed to use IEEE 128-bit floating point

[Michael Meissner]

On Tue, Oct 23, 2018 at 04:18:55PM -0500, Segher Boessenkool wrote:
> Hi Mike,
> 
> On Tue, Oct 23, 2018 at 04:12:11PM -0400, Michael Meissner wrote:
> > This patch changes the name used by the <math>l built-in functions that return
> > or are passed long double if the long double type is changed from the current
> > IBM long double format to the IEEE 128-bit format.
> > 
> > I have done a bootstrap and make check with no regressions on a little endian
> > power8 system.  Is it ok to check into the trunk?  This will need to be back
> > ported to the GCC 8.x branch.
> 
> Could you test it on the usual assortment of systems instead of just one?
> BE 7 and 8, LE 8 and 9.  Or wait for possible fallout :-)

Ok.

> A backport to 8 is wanted yes, but please wait as usual.  It's fine after
> a week or so of no problems.

Yep.

> 
> > +/* On 64-bit Linux and Freebsd systems, possibly switch the long double library
> > +   function names from <foo>l to <foo>f128 if the default long double type is
> > +   IEEE 128-bit.  Typically, with the C and C++ languages, the standard math.h
> > +   include file switches the names on systems that support long double as IEEE
> > +   128-bit, but that doesn't work if the user uses __builtin_<foo>l directly or
> > +   if they use Fortran.  Use the TARGET_MANGLE_DECL_ASSEMBLER_NAME hook to
> > +   change this name.  We only do this if the default is long double is not IEEE
> > +   128-bit, and the user asked for IEEE 128-bit.  */
> 
> s/default is/default/
> 
> Does this need some synchronisation with the libm headers?  I guess things
> will just work out, but it is desirable if libm stops doing this with
> compilers that have this change?

It should just work out assuming you are using a recent enough GLIBC.  The
patch is more for when you aren't using headers.

> > +static tree
> > +rs6000_mangle_decl_assembler_name (tree decl, tree id)
> > +{
> > +  if (!TARGET_IEEEQUAD_DEFAULT && TARGET_IEEEQUAD && TARGET_LONG_DOUBLE_128
> 
> Write this is in the opposite order?
>   if (TARGET_LONG_DOUBLE_128 && TARGET_IEEEQUAD && !TARGET_IEEEQUAD_DEFAULT

Because !TARGET_IEEEQUAD_DEFAULT is a constant test.  If you are on a system
that defaults to IEEE 128-bit, the whole code gets deleted.  I would hope the
tests still get deleted if it occurs later in the test, but I tend to put the
things that can be optimized at compile time first.

> > +    {
> > +      size_t len = IDENTIFIER_LENGTH (id);
> > +      const char *name = IDENTIFIER_POINTER (id);
> > +
> > +      if (name[len-1] == 'l')
> > +	{
> > +	  bool has_long_double_p = false;
> > +	  tree type = TREE_TYPE (decl);
> > +	  machine_mode ret_mode = TYPE_MODE (type);
> > +
> > +	  /* See if the function returns long double or long double
> > +	     complex.  */
> > +	  if (ret_mode == TFmode || ret_mode == TCmode)
> > +	    has_long_double_p = true;
> 
> This comment is a bit misleading I think?  The code checks if it is the
> same mode as would be used for long double, not if that is the actual
> asked-for type.  The code is fine AFAICS, the comment isn't so great
> though.

Well the long double type is 'TFmode'.  Though _Float128 does get mapped to
TFmode instead of KFmode also.  But explicit f128 built-ins won't go through
here, since they don't end in 'l'.  I'm just trying to avoid things like CLZL
that take long arguments and not long double.

> > +	  else
> > +	    {
> > +	      function_args_iterator args_iter;
> > +	      tree arg;
> > +
> > +	      /* See if we have a long double or long double complex
> > +		 argument.  */
> 
> And same here.
> 
> > +	      FOREACH_FUNCTION_ARGS (type, arg, args_iter)
> > +		{
> > +		  machine_mode arg_mode = TYPE_MODE (arg);
> > +		  if (arg_mode == TFmode || arg_mode == TCmode)
> > +		    {
> > +		      has_long_double_p = true;
> > +		      break;
> > +		    }
> > +		}
> > +	    }
> > +
> > +	  /* If we have long double, change the name.  */
> 
> And this.
> 
> > +	  if (has_long_double_p)
> > +	    {
> > +	      char *name2 = (char *) alloca (len + 4);
> > +	      memcpy (name2, name, len-1);
> 
> len - 1

Ok.

> Okay for trunk with those things fixed.  Thanks!
> 
> 
> Segher
> 

-- 
Michael Meissner, IBM
IBM, M/S 2506R, 550 King Street, Littleton, MA 01460-6245, USA
email: meissner@linux.ibm.com, phone: +1 (978) 899-4797