[Bug target/104172] New: [9/10/11/12 Regression] ppc64le mangling ICE with -flto -ffat-lto-objects

[Bug target/104172] New: [9/10/11/12 Regression] ppc64le mangling ICE with -flto -ffat-lto-objects

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104172

            Bug ID: 104172
           Summary: [9/10/11/12 Regression] ppc64le mangling ICE with
                    -flto -ffat-lto-objects
           Product: gcc
           Version: 12.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: target
          Assignee: unassigned at gcc dot gnu.org
          Reporter: jakub at gcc dot gnu.org
  Target Milestone: ---

// { dg-do compile { target { powerpc*-*-* && lp64 && le } } }
// { dg-options "-O2 -flto=auto -ffat-lto-objects -mabi=ieeelongdouble" }

struct A { A(int); };
struct B { float b; };
struct C : B { C(); };
C::C() { A(b * 25.4L); }

ICEs
PagePrinter.C: In member function 'C::C()':
PagePrinter.C:7:1: internal compiler error: tree check: expected class 'type',
have 'declaration' (translation_unit_decl) in base_ctor_omit_inherited_parms,
at cp/method.c:568
    7 | C::C() { A(b * 25.4L); }
      | ^
0x101fbd8b tree_class_check_failed(tree_node const*, tree_code_class, char
const*, int, char const*)
        ../../gcc/tree.c:8752
0x104073bf tree_class_check(tree_node*, tree_code_class, char const*, int, char
const*)
        ../../gcc/tree.h:3564
0x104073bf base_ctor_omit_inherited_parms(tree_node*)
        ../../gcc/cp/method.c:568
0x104073bf base_ctor_omit_inherited_parms(tree_node*)
        ../../gcc/cp/method.c:560
0x103eda0b write_method_parms
        ../../gcc/cp/mangle.c:2798
0x103edd73 write_bare_function_type
        ../../gcc/cp/mangle.c:2758
0x103edecf write_encoding
        ../../gcc/cp/mangle.c:832
0x103eec7f mangle_decl_string
        ../../gcc/cp/mangle.c:4034
0x103ef003 get_mangled_id
        ../../gcc/cp/mangle.c:4055
0x103ef003 mangle_decl(tree_node*)
        ../../gcc/cp/mangle.c:4093
0x1142297f rs6000_globalize_decl_name
        ../../gcc/config/rs6000/rs6000.c:28190
0x113d8c43 globalize_decl
        ../../gcc/varasm.c:6146
0x113e112b assemble_start_function(tree_node*, char const*)
        ../../gcc/varasm.c:1965
0x10a01a8f rest_of_handle_final
        ../../gcc/final.c:4281
0x10a01a8f execute
        ../../gcc/final.c:4363
(for GCC configured to default to --with-long-double-format=ieee that is a
pretty serious regression breaking lots of builds).

The bug is in:
/* Create an alias for a mangled name where we have changed the mangling (in
   GCC 8.1, we used U10__float128, and now we use u9__ieee128).  This is called
   via the target hook TARGET_ASM_GLOBALIZE_DECL_NAME.  */

#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 (rs6000_passes_ieee128 && name[0] == '_' && name[1] == 'Z')
    {
      tree save_asm_name = DECL_ASSEMBLER_NAME (decl);
      const char *old_name;

      ieee128_mangling_gcc_8_1 = true;
      lang_hooks.set_decl_assembler_name (decl);
      old_name = IDENTIFIER_POINTER (DECL_ASSEMBLER_NAME (decl));
      SET_DECL_ASSEMBLER_NAME (decl, save_asm_name);
      ieee128_mangling_gcc_8_1 = false;

      if (strcmp (name, old_name) != 0)
        {
          fprintf (stream, "\t.weak %s\n", old_name);
          fprintf (stream, "\t.set %s,%s\n", old_name, name);
        }
    }
}
#endif

This is just wrong, mangling is only possible before free_lang_data, not after
that, everything should be mangled by then.
So, IMHO the backend should note that during mangling that it emitted some
u9__ieee128 (when rs6000_mangle_type returns it)
and through some hook should cooperate with the C++ FE to create a mangling
alias like the FE creates mangling aliases for -fabi-version*.

[Bug target/104172] [9/10/11/12 Regression] ppc64le mangling ICE with -flto -ffat-lto-objects

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104172

Jakub Jelinek <jakub at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |dje at gcc dot gnu.org,
                   |                            |hubicka at gcc dot gnu.org,
                   |                            |jason at gcc dot gnu.org,
                   |                            |meissner at gcc dot gnu.org,
                   |                            |segher at gcc dot gnu.org
           Priority|P3                          |P2
           Keywords|                            |ice-on-valid-code,
                   |                            |wrong-code
   Target Milestone|---                         |9.5

--- Comment #1 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Wonder what it does in lto1 (e.g. without -ffat-lto-objects) if adapted such
that it actually links.  Because lto1 certainly doesn't have the C++ mangler,
so it must be emitting something really weird in that case.

[Bug target/104172] [9/10/11/12 Regression] ppc64le mangling ICE with -flto -ffat-lto-objects

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104172

--- Comment #2 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Was -mabi=ieeelongdouble support even remotely usable in GCC 8 (do we really
need to care about compatibility with it)?

[Bug target/104172] [9/10/11/12 Regression] ppc64le mangling ICE with -flto -ffat-lto-objects

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104172

--- Comment #3 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
E.g. I think all the libstdc++ work is only in GCC 11, so yes, one could use
C++, but couldn't use the standard C++ library in GCC 8.

[Bug target/104172] [9/10/11/12 Regression] ppc64le mangling ICE with -flto -ffat-lto-objects

[mpolacek at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104172

Marek Polacek <mpolacek at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |NEW
                 CC|                            |mpolacek at gcc dot gnu.org
   Last reconfirmed|                            |2022-01-21
     Ever confirmed|0                           |1

--- Comment #4 from Marek Polacek <mpolacek at gcc dot gnu.org> ---
Confirmed.

[Bug target/104172] [9/10/11/12 Regression] ppc64le mangling ICE with -flto -ffat-lto-objects

[wschmidt at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104172

Bill Schmidt <wschmidt at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |wschmidt at gcc dot gnu.org

--- Comment #5 from Bill Schmidt <wschmidt at gcc dot gnu.org> ---
(In reply to Jakub Jelinek from comment #3)
> E.g. I think all the libstdc++ work is only in GCC 11, so yes, one could use
> C++, but couldn't use the standard C++ library in GCC 8.

Yes, that's true.

[Bug target/104172] [9/10/11/12 Regression] ppc64le mangling ICE with -flto -ffat-lto-objects

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104172

--- Comment #6 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
>From my side, I think best would be just to partially revert
r9-1033-gc765c8cb73b0bb91e9f0401ab6567b6218b910e5
(all the rs6000_passes_ieee128 and ieee128_mangling_gcc_8_1 related stuff in
there, don't set those and when testing assume false).
Rationale, -mabi=ieeelongdouble has been an undocumented unsupported option
before GCC 8, it is true it was "supported" option in 8.1, but still far from
being fully usable both because of the missing libstdc++ support and important
bugfixes.  And in 8.2 the ABI we have now has been backported -
r8-8265-gd8f96ab114a977d9 .  So, we are taking a lot of pain for compatibility
with something that likely nobody used, glibc support wasn't there until the
2.32 release done after 8.2 was released.

If not and we do want the pain, then all the mangling has to be done before
free_lang_data (so can't be driven by rs6000_passes_ieee128 which is RTL-ish
time, but on whether u9__ieee128 has been emitted into the mangled name),
ideally through a target hook in mangle_decl where it also does the
          record_mangling (decl, G.need_abi_warning);

          if (!G.need_abi_warning)
            return;

          flag_abi_version = flag_abi_compat_version;
          id2 = mangle_decl_string (decl);
          id2 = targetm.mangle_decl_assembler_name (decl, id2);
          flag_abi_version = save_ver;

          if (id2 != id)
            note_mangling_alias (decl, id2);
stuff (perhaps right before that if (!G.need_abi_warning) spot).

Note, some Fedora packages fail with weirdo errors like:
{standard input}: Assembler messages:
{standard input}:43720: Error: junk at end of line, first unrecognized
character is `['
{standard input}:43721: Error: expected comma after "operator"
{standard input}:45644: Error: junk at end of line, first unrecognized
character is `.'
{standard input}:45645: Error: expected comma after "__ct_base"
{standard input}:46051: Error: junk at end of line, first unrecognized
character is `.'
{standard input}:46052: Error: expected comma after "__ct_base"
{standard input}:46794: Error: junk at end of line, first unrecognized
character is `.'
{standard input}:46795: Error: expected comma after "__ct_base"
{standard input}:48803: Error: junk at end of line, first unrecognized
character is `='
{standard input}:48804: Error: expected comma after "operator"
{standard input}:49232: Error: junk at end of line, first unrecognized
character is `.'
{standard input}:49233: Error: expected comma after "__ct_base"
and I strongly believe it is the same thing, lto1 and the identifier being
mangled in a weirdo way.

[Bug target/104172] [9/10/11/12 Regression] ppc64le mangling ICE with -flto -ffat-lto-objects

[orion at cora dot nwra.com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104172

Orion Poplawski <orion at cora dot nwra.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |orion at cora dot nwra.com

--- Comment #7 from Orion Poplawski <orion at cora dot nwra.com> ---
I believe this is causing the ccache compile failure in Fedora rawhide:
https://koji.fedoraproject.org/koji/buildinfo?buildID=1881086

[Bug target/104172] [9/10/11/12 Regression] ppc64le mangling ICE with -flto -ffat-lto-objects

[rguenth at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104172

Richard Biener <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Target|                            |powerpc64le

--- Comment #8 from Richard Biener <rguenth at gcc dot gnu.org> ---
lto1 sees pre-mangled symbols

[Bug target/104172] [9/10/11/12 Regression] ppc64le mangling ICE with -flto -ffat-lto-objects

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104172

--- Comment #9 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
That is what it should do.  But it also has a mangling langhook and the backend
uses it in the hope that it is a C++ FE langhook that will mangle the names
like they were previously except with the s/u9__ieee128/U10__float128/g
changes, but it of course doesn't do that.

[Bug target/104172] [9/10/11/12 Regression] ppc64le mangling ICE with -flto -ffat-lto-objects

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104172

--- Comment #10 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Created attachment 52276
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=52276&action=edit
gcc12-pr104172.patch

Untested patch to do the mangling aliases inside of the C++ FE.
My preference is still to remove that 8.1 compatibility support, this is just
in case Michael and the IBM backend maintainers strongly prefer to keep it.
If we keep it, an open question is about the other C++ FE mangling aliases,
shall we also do these backend specific mangling aliases on top of those
-fabi-compat-version= compatibility aliases?

[Bug target/104172] [9/10/11/12 Regression] ppc64le mangling ICE with -flto -ffat-lto-objects

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104172

--- Comment #11 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Created attachment 52279
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=52279&action=edit
gcc12-pr104172.patch

Variant patch that just removes the GCC 8.1 compatibility mangling aliases.

[Bug target/104172] [9/10/11/12 Regression] ppc64le mangling ICE with -flto -ffat-lto-objects

[jason at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104172

--- Comment #12 from Jason Merrill <jason at gcc dot gnu.org> ---
I'm persuaded by your argument that we don't need to care about compatibility
with the 8.1 handling of these types, which weren't yet really usable, so we
can go with the simpler patch.  But I'd like to hear from IBM.

[Bug target/104172] [9/10/11/12 Regression] ppc64le mangling ICE with -flto -ffat-lto-objects

[wschmidt at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104172

--- Comment #13 from Bill Schmidt <wschmidt at gcc dot gnu.org> ---
We discussed this with Jakub today, and we concur.

[Bug target/104172] [9/10/11/12 Regression] ppc64le mangling ICE with -flto -ffat-lto-objects

[segher at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104172

--- Comment #14 from Segher Boessenkool <segher at gcc dot gnu.org> ---
I already approved the patch.

[Bug target/104172] [9/10/11/12 Regression] ppc64le mangling ICE with -flto -ffat-lto-objects

[cvs-commit at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104172

--- Comment #15 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Jakub Jelinek <jakub@gcc.gnu.org>:

https://gcc.gnu.org/g:f4ee27d3262fc4fc3e5d3535f195fdcf87d7ec77

commit r12-6853-gf4ee27d3262fc4fc3e5d3535f195fdcf87d7ec77
Author: Jakub Jelinek <jakub@redhat.com>
Date:   Tue Jan 25 05:49:05 2022 +0100

    rs6000: Remove GCC 8.1 U10__float128 mangling compatibility [PR104172]

    In GCC 7.x and earlier, while it had -mabi=ieeelongdouble option, that
option
    was undocumented and unsupported.
    In GCC 8.1 that option got documented and -mabi=ieeelongdouble long double
started
    to be mangled as U10__float128.
    In GCC 9 and backported to before 8.2 release, that mangling changed to
    u9__ieee128 and a support for emitting compatibility mangling aliases have
    been added.
    Unfortunately, as mentioned in the PR, those don't really work well in many
    cases, the free_lang_data pass throws away important trees, so e.g. with
    -flto -ffat-lto-objects the compiler often ICEs on templates that involve
    IEEE quad long double arguments etc. because the mangling was done too late
    (at final time).
    Furthermore, lto1's mangler is not the C++ mangler, so with -flto it would
    often emit as "mangled identifiers" something that wasn't a valid assembler
    identifier, e.g. operator+ etc.
    While it is possible to do such mangling earlier, e.g. at the same time
when
    the C++ FE emits its mangling aliases and untested proof of concept is in
    the PR, there seems to be agreement that we shouldn't bother with this
    ABI compatibility with something that probably nobody really used.
    GCC 8.2 already uses the new mangling, it was just a few months, but more
    importantly, libstdc++ support for IEEE quad long double on
    powerpc64le-linux was only added in GCC 11, and glibc support for that some
    weeks after 8.2 got released.

    So, the following patch just drops those aliases.

    2022-01-25  Jakub Jelinek  <jakub@redhat.com>

            PR target/104172
    gcc/
            * config/rs6000/rs6000-internal.h (rs6000_passes_ieee128): Don't
            declare.
            * config/rs6000/rs6000.cc (rs6000_passes_ieee128,
            ieee128_mangling_gcc_8_1): Remove.
            (TARGET_ASM_GLOBALIZE_DECL_NAME): Don't redefine.
            (rs6000_mangle_type): Return "u9__ieee128" instead of
            ieee128_mangling_gcc_8_1 ? "U10__float128" : "u9__ieee128".
            (rs6000_globalize_decl_name): Remove.
            * config/rs6000/rs6000-call.cc (init_cumulative_args,
            rs6000_function_arg_advance_1): Don't set rs6000_passes_ieee128.

[Bug target/104172] [9/10/11/12 Regression] ppc64le mangling ICE with -flto -ffat-lto-objects

[cvs-commit at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104172

--- Comment #16 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The releases/gcc-11 branch has been updated by Jakub Jelinek
<jakub@gcc.gnu.org>:

https://gcc.gnu.org/g:79b0091b13eb7dce0294407d9bd78750df10180d

commit r11-9510-g79b0091b13eb7dce0294407d9bd78750df10180d
Author: Jakub Jelinek <jakub@redhat.com>
Date:   Tue Jan 25 05:49:05 2022 +0100

    rs6000: Remove GCC 8.1 U10__float128 mangling compatibility [PR104172]

    In GCC 7.x and earlier, while it had -mabi=ieeelongdouble option, that
option
    was undocumented and unsupported.
    In GCC 8.1 that option got documented and -mabi=ieeelongdouble long double
started
    to be mangled as U10__float128.
    In GCC 9 and backported to before 8.2 release, that mangling changed to
    u9__ieee128 and a support for emitting compatibility mangling aliases have
    been added.
    Unfortunately, as mentioned in the PR, those don't really work well in many
    cases, the free_lang_data pass throws away important trees, so e.g. with
    -flto -ffat-lto-objects the compiler often ICEs on templates that involve
    IEEE quad long double arguments etc. because the mangling was done too late
    (at final time).
    Furthermore, lto1's mangler is not the C++ mangler, so with -flto it would
    often emit as "mangled identifiers" something that wasn't a valid assembler
    identifier, e.g. operator+ etc.
    While it is possible to do such mangling earlier, e.g. at the same time
when
    the C++ FE emits its mangling aliases and untested proof of concept is in
    the PR, there seems to be agreement that we shouldn't bother with this
    ABI compatibility with something that probably nobody really used.
    GCC 8.2 already uses the new mangling, it was just a few months, but more
    importantly, libstdc++ support for IEEE quad long double on
    powerpc64le-linux was only added in GCC 11, and glibc support for that some
    weeks after 8.2 got released.

    So, the following patch just drops those aliases.

    2022-01-25  Jakub Jelinek  <jakub@redhat.com>

            PR target/104172
    gcc/
            * config/rs6000/rs6000-internal.h (rs6000_passes_ieee128): Don't
            declare.
            * config/rs6000/rs6000.c (rs6000_passes_ieee128,
            ieee128_mangling_gcc_8_1): Remove.
            (TARGET_ASM_GLOBALIZE_DECL_NAME): Don't redefine.
            (rs6000_mangle_type): Return "u9__ieee128" instead of
            ieee128_mangling_gcc_8_1 ? "U10__float128" : "u9__ieee128".
            (rs6000_globalize_decl_name): Remove.
            * config/rs6000/rs6000-call.c (init_cumulative_args,
            rs6000_function_arg_advance_1): Don't set rs6000_passes_ieee128.

    (cherry picked from commit f4ee27d3262fc4fc3e5d3535f195fdcf87d7ec77)

[Bug target/104172] [9/10 Regression] ppc64le mangling ICE with -flto -ffat-lto-objects

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104172

Jakub Jelinek <jakub at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
            Summary|[9/10/11/12 Regression]     |[9/10 Regression] ppc64le
                   |ppc64le mangling ICE with   |mangling ICE with -flto
                   |-flto -ffat-lto-objects     |-ffat-lto-objects
   Target Milestone|9.5                         |11.3
             Status|NEW                         |RESOLVED
         Resolution|---                         |FIXED

--- Comment #17 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Fixed for 11.3/trunk, not going to backport further.