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: meiss...@linux.ibm.com, phone: +1 (978) 899-4797
