On Mon, Jun 04, 2018 at 12:46:42PM -0500, Segher Boessenkool wrote:
> Hi Mike,
>
> On Fri, Jun 01, 2018 at 07:28:40PM -0400, Michael Meissner wrote:
> > This patch also makes __ibm128 or __float128 use the long double mode if
> > long
> > double uses the IBM extended double or IEEE 128-bit representations.
>
> Does that need to be the same patch? Please split such things out when
> you can.
Well it can be split, but it will then have the test failure still.
> > This
> > allows templates to work again with those types (the template code aborts if
> > you have two distinct types use the same mangling). However, overloaded
> > types
> > won't work, but I suspect these days people use templates over overloaded
> > functions. I fixed up the test case for this (pr85657.C) so that it only
> > tests
> > for templates.
>
> If tests fail, do not delete the test. If we have a problem, we have
> a problem, and it needs to be fixed (not necessarily today of course,
> and there is xfail for long-standing problems).
This is a fundamental detail of the current changes. There is no way it can be
'fixed'.
If __float128/__ibm128 each use the long double type internally when long
double uses that representation, then you cannot have overloaded functions that
use the two types.
I.e.
class foo {
// ...
long double arith (long double);
__float128 arith (__float128);
__ibm128 arith (__ibm128);
}
In the previous changes that the test was written for, we had 3 types within
the compiler. We had a __float128 type, we had a long double type, and we had
an __ibm128 type. We had different manglings for each of the different types.
Now that we only have two types, you can't have explicit overloading of the
same type. You can have templates, because only one version of the template is
created for the two types.
The test in question was new and written when I did the previous changes. It
is not a long standing problem. The test was explicitly written to make sure
all three types were different. Since we now have only two types, we need to
adjust the test.
>
> > +/* Generate old manged name, not new name. */
> > +static bool old_mangling;
>
> As Andreas said, this is not a good name. Please at least mention for
> what type this old mangling is.
I will consider this.
>
> > @@ -16355,14 +16394,21 @@ rs6000_init_builtins (void)
> > __ieee128. */
> > if (TARGET_FLOAT128_TYPE)
> > {
> > - ibm128_float_type_node = make_node (REAL_TYPE);
> > - TYPE_PRECISION (ibm128_float_type_node) = 128;
> > - SET_TYPE_MODE (ibm128_float_type_node, IFmode);
> > - layout_type (ibm128_float_type_node);
> > + if (TARGET_IEEEQUAD || !TARGET_LONG_DOUBLE_128)
> > + {
> > + ibm128_float_type_node = make_node (REAL_TYPE);
> > + TYPE_PRECISION (ibm128_float_type_node) = 128;
> > + SET_TYPE_MODE (ibm128_float_type_node, IFmode);
> > + layout_type (ibm128_float_type_node);
> > + }
> > + else
> > + ibm128_float_type_node = long_double_type_node;
>
> I wonder how hard it would be to alias the long double type to either
> __ibm128 or __ieee128, instead of the other way around? This would
> simplify code a lot. Esp. if we can do the same thing for the modes,
> too.
It depends on whether other parts of the compiler already have links to long
double before the hook in rs6000.c gets called. I frankly don't see it as
simplifying the code.
>
> > @@ -32117,7 +32163,7 @@ rs6000_mangle_type (const_tree type)
> > if (SCALAR_FLOAT_TYPE_P (type) && FLOAT128_IBM_P (TYPE_MODE (type)))
> > return "g";
> > if (SCALAR_FLOAT_TYPE_P (type) && FLOAT128_IEEE_P (TYPE_MODE (type)))
> > - return "u9__ieee128";
> > + return old_mangling ? "U10__float128" : "u9__ieee128";
>
> > +#if TARGET_ELF && RS6000_WEAK
> > +static void
> > +rs6000_globalize_decl_name (FILE * stream, tree decl)
> > +{
> > + const char *name = XSTR (XEXP (DECL_RTL (decl), 0), 0);
> > +
> > + targetm.asm_out.globalize_label (stream, name);
> > +
> > + if (TARGET_OLD_NAME_MANGLING && rs6000_passes_ieee128
> > + && name[0] == '_' && name[1] == 'Z')
> > + {
> > + tree save_asm_name = DECL_ASSEMBLER_NAME (decl);
> > + const char *old_name;
> > +
> > + old_mangling = true;
> > + lang_hooks.set_decl_assembler_name (decl);
> > + old_name = IDENTIFIER_POINTER (DECL_ASSEMBLER_NAME (decl));
> > + SET_DECL_ASSEMBLER_NAME (decl, save_asm_name);
> > + old_mangling = false;
>
> Eww. Can't we just create old_name directly? Maybe there already is
> some nice helper for this?
No, there is no way to do this, without having to write a parser for the
mangled names to change the names. I actually looked at that, and it was
getting to be complex. In addition, it is fragile, in that if C++ decides to
tweak the mangling ABI (like they did in C++-17 if I read the code correctly),
we would have to make parallel changes in the PowerPC.
> > +;; -malias-old-name-mangling: implement an alias to map old C++ mangled
> > names
> > +;; to the new version of the name
> > +malias-old-name-mangling
> > +Target Undocumented Var(TARGET_OLD_NAME_MANGLING) Init(-1) Save
>
> As said, better name please. Or, just drop the option (always enable it).
I will ponder what to call the internal variable, and whether to delete the
option.
--
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
