PowerPC: Map IEEE 128-bit long double built-in functions

[Michael Meissner]

On Wed, Nov 04, 2020 at 06:13:57PM -0600, Segher Boessenkool wrote:
> Hi!
> 
> On Thu, Oct 22, 2020 at 06:03:46PM -0400, Michael Meissner wrote:
> > To map the scanf functions, <name> is mapped to __isoc99_<name>ieee128.
> 
> Is that correct?  What if you are compiling for c90?

That is the name in GLIBC.

> > 	* config/rs6000/rs6000.c (rs6000_mangle_decl_assembler_name): Add
> > 	support for mapping built-in function names for long double
> > 	built-in functions if long double is IEEE 128-bit.
> 
> "Map the built-in function names" etc.
> 
> > --- a/gcc/config/rs6000/rs6000.c
> > +++ b/gcc/config/rs6000/rs6000.c
> > @@ -26893,56 +26893,127 @@ rs6000_globalize_decl_name (FILE * stream, tree decl)
> >     library before you can switch the real*16 type at compile time.
> >  
> >     We use the TARGET_MANGLE_DECL_ASSEMBLER_NAME hook to change this name.  We
> > -   only do this if the default is that long double is IBM extended double, and
> > -   the user asked for IEEE 128-bit.  */
> > +   only do this transformation if the __float128 type is enabled.  This
> > +   prevents us from doing the transformation on older 32-bit ports that might
> > +   have enabled using IEEE 128-bit floating point as the default long double
> > +   type.  */
> 
> I don't see why that is the right thing to do?  You'll have exactly
> these same problems on 32-bit!

The comment is refering to the remote possiblity that 32-bit VxWorks was
configured to use IEEE long double as the only long double type.  However, you
had to use several non-default options to do this.  All other historic 32-bit
implementations did not allow IEEE long double.

> 
> Hrm, we talked about that before I guess?  Do you just need to change
> this comment now?
> 
> > +	default:
> > +	  break;
> > +	}
> 
> That is useless, just leave it out?  The end of a switch will always
> fall through, and it is normal idiom to use that.

And it will get a warning that some enumeration elements did not have case
elements.

> > +      /* Update the __builtin_*printf && __builtin_*scanf functions.  */
> 
> "and" :-)
> 
> > +	  else if (name[len - 1] == 'l')
> > +	    {
> > +	      bool uses_ieee128_p = false;
> > +	      tree type = TREE_TYPE (decl);
> > +	      machine_mode ret_mode = TYPE_MODE (type);
> > +
> > +	      /* See if the function returns a IEEE 128-bit floating point type or
> > +		 complex type.  */
> > +	      if (ret_mode == TFmode || ret_mode == TCmode)
> > +		uses_ieee128_p = true;
> > +	      else
> >  		{
> > -		  machine_mode arg_mode = TYPE_MODE (arg);
> > -		  if (arg_mode == TFmode || arg_mode == TCmode)
> > +		  function_args_iterator args_iter;
> > +		  tree arg;
> 
> (declare that right before the FOREACH)
> 
> > +
> > +		  /* See if the function passes a IEEE 128-bit floating point type
> > +		     or complex type.  */
> > +		  FOREACH_FUNCTION_ARGS (type, arg, args_iter)
> >  		    {
> > -		      uses_ieee128_p = true;
> > -		      break;
> > +		      machine_mode arg_mode = TYPE_MODE (arg);
> > +		      if (arg_mode == TFmode || arg_mode == TCmode)
> > +			{
> > +			  uses_ieee128_p = true;
> > +			  break;
> > +			}
> >  		    }
> >  		}
> 
> There is no point in doing all these early-outs in an initialisation
> function, making it much harder to read :-(
> 
> > +	      /* If we passed or returned an IEEE 128-bit floating point type,
> > +		 change the name.  Use __<name>ieee128, instead of <name>l.  */
> > +	      if (uses_ieee128_p)
> > +		newname = xasprintf ("__%.*sieee128", (int)(len - 1), name);
> 
> (int) (len - 1)
> 
> Please comment what the - 1 does, and/or what this is for at all.  (In
> the code / in a comment, not to me, I figured it out after a while.)

The comment just above the line says what it does.  I.e. it does not copy the
'l' in the name (i.e. sinl).

-- 
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