[Bug target/99216] New: ICE in aarch64_sve::function_expander::expand() with LTO

[Bug target/99216] New: ICE in aarch64_sve::function_expander::expand() with LTO

[acoplan at gcc dot gnu.org]

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

            Bug ID: 99216
           Summary: ICE in aarch64_sve::function_expander::expand() with
                    LTO
           Product: gcc
           Version: 11.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: target
          Assignee: unassigned at gcc dot gnu.org
          Reporter: acoplan at gcc dot gnu.org
  Target Milestone: ---

The following fails:

$ cat test.cc
#include <arm_sve.h>
bool a;
int main() { a = svaddv(svptrue_b8(), svdup_s8(0)); }

$ aarch64-linux-gnu-gcc -march=armv8.2-a+sve -flto test.cc
during RTL pass: expand
test.cc: In function ‘main’:
test.cc:3:24: internal compiler error: in operator[], at vec.h:890
    3 | int main() { a = svaddv(svptrue_b8(), svdup_s8(0)); }
      |                        ^
0x11a1562 vec<rtx_def*, va_heap, vl_embed>::operator[](unsigned int)
        /home/alecop01/toolchain/src/gcc/gcc/vec.h:890
0x11a1562 vec<rtx_def*, va_heap, vl_ptr>::operator[](unsigned int)
        /home/alecop01/toolchain/src/gcc/gcc/vec.h:1461
0x11a1562 expand
       
/home/alecop01/toolchain/src/gcc/gcc/config/aarch64/aarch64-sve-builtins-base.cc:237
0x1196101 aarch64_sve::function_expander::expand()
       
/home/alecop01/toolchain/src/gcc/gcc/config/aarch64/aarch64-sve-builtins.cc:3318
0x1197242 aarch64_sve::expand_builtin(unsigned int, tree_node*, rtx_def*)
       
/home/alecop01/toolchain/src/gcc/gcc/config/aarch64/aarch64-sve-builtins.cc:3607
0x10f62eb aarch64_expand_builtin
        /home/alecop01/toolchain/src/gcc/gcc/config/aarch64/aarch64.c:13568
0x74b79a expand_builtin(tree_node*, rtx_def*, rtx_def*, machine_mode, int)
        /home/alecop01/toolchain/src/gcc/gcc/builtins.c:9518
0x8c51cc expand_expr_real_1(tree_node*, rtx_def*, machine_mode,
expand_modifier, rtx_def**, bool)
        /home/alecop01/toolchain/src/gcc/gcc/expr.c:11276
0x8c6779 expand_expr_real(tree_node*, rtx_def*, machine_mode, expand_modifier,
rtx_def**, bool)
        /home/alecop01/toolchain/src/gcc/gcc/expr.c:8513
0x8cfc1c store_expr(tree_node*, rtx_def*, int, bool, bool)
        /home/alecop01/toolchain/src/gcc/gcc/expr.c:5886
0x8d1927 expand_assignment(tree_node*, tree_node*, bool)
        /home/alecop01/toolchain/src/gcc/gcc/expr.c:5622
0x780d7c expand_call_stmt
        /home/alecop01/toolchain/src/gcc/gcc/cfgexpand.c:2838
0x780d7c expand_gimple_stmt_1
        /home/alecop01/toolchain/src/gcc/gcc/cfgexpand.c:3844
0x780d7c expand_gimple_stmt
        /home/alecop01/toolchain/src/gcc/gcc/cfgexpand.c:4008
0x78a5a7 expand_gimple_basic_block
        /home/alecop01/toolchain/src/gcc/gcc/cfgexpand.c:6045
0x78bc61 execute
        /home/alecop01/toolchain/src/gcc/gcc/cfgexpand.c:6729
Please submit a full bug report,
with preprocessed source if appropriate.
Please include the complete backtrace with any bug report.
See <https://gcc.gnu.org/bugs/> for instructions.
lto-wrapper: fatal error: aarch64-linux-gnu-gcc returned 1 exit status
compilation terminated.
/home/alecop01/toolchain/build-aarch64-linux-gnu/install/bin/../lib/gcc/aarch64-linux-gnu/11.0.0/../../../../aarch64-linux-gnu/bin/ld:
error: lto-wrapper failed
collect2: error: ld returned 1 exit status

[Bug target/99216] ICE in aarch64_sve::function_expander::expand() with LTO

[ktkachov at gcc dot gnu.org]

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

ktkachov at gcc dot gnu.org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|---                         |10.3
     Ever confirmed|0                           |1
      Known to fail|                            |10.2.1
                 CC|                            |ktkachov at gcc dot gnu.org,
                   |                            |rsandifo at gcc dot gnu.org
   Last reconfirmed|                            |2021-02-23
             Status|UNCONFIRMED                 |NEW

--- Comment #1 from ktkachov at gcc dot gnu.org ---
Confirmed. ICEs on GCC 10 as well

[Bug target/99216] ICE in aarch64_sve::function_expander::expand() with LTO

[acoplan at gcc dot gnu.org]

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

--- Comment #2 from Alex Coplan <acoplan at gcc dot gnu.org> ---
This one ICEs at all optimization levels (not just -O0):

#include <arm_sve.h>
unsigned long long a;
void b(unsigned long long *c, int e) { *c ^= e; }
bool d;
int main() {
  d = svaddv(svptrue_pat_b8(SV_VL16), svdup_s8(0));
  b(&a, d);
}

[Bug target/99216] ICE in aarch64_sve::function_expander::expand() with LTO

[acoplan at gcc dot gnu.org]

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

Alex Coplan <acoplan at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |ASSIGNED
           Assignee|unassigned at gcc dot gnu.org      |acoplan at gcc dot gnu.org

--- Comment #3 from Alex Coplan <acoplan at gcc dot gnu.org> ---
I'll take a look

[Bug target/99216] ICE in aarch64_sve::function_expander::expand() with LTO

[acoplan at gcc dot gnu.org]

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

--- Comment #4 from Alex Coplan <acoplan at gcc dot gnu.org> ---
Right, the problem appears to be to do with the way that overloaded functions
are implemented for the ACLE. Specifically the m_direct_overloads flag in
aarch64_sve::function_builder. If this flag is set, we register a separate
builtin (with a separate function code) for each overload as opposed to
registering the overloaded function once and resolving it later. The two
different schemes end up with each builtin having a different code.

We set m_direct_overloads to be true if the language is C++:

m_direct_overloads = lang_GNU_CXX ();

so in cc1plus, we use one numbering scheme, but in lto1, we use a different
numbering scheme, with predictably disastrous consequences (we try and expand
svaddv as an svbic).

So one options would be that for LTO we instantiate both sets of tree nodes.
Then, when expanding a tree node that came from LTO, we dispatch on a flag in
the tree node (essentially just whether it came from C++ or not) to determine
which set of functions to use. Seems a bit messy though.

@Richard: does that sound at all sane? Any ideas for a better approach?

[Bug target/99216] ICE in aarch64_sve::function_expander::expand() with LTO

[rsandifo at gcc dot gnu.org]

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

--- Comment #5 from rsandifo at gcc dot gnu.org <rsandifo at gcc dot gnu.org> ---
(In reply to Alex Coplan from comment #4)
> Right, the problem appears to be to do with the way that overloaded
> functions are implemented for the ACLE. Specifically the m_direct_overloads
> flag in aarch64_sve::function_builder. If this flag is set, we register a
> separate builtin (with a separate function code) for each overload as
> opposed to registering the overloaded function once and resolving it later.
> The two different schemes end up with each builtin having a different code.
> 
> We set m_direct_overloads to be true if the language is C++:
> 
> m_direct_overloads = lang_GNU_CXX ();
> 
> so in cc1plus, we use one numbering scheme, but in lto1, we use a different
> numbering scheme, with predictably disastrous consequences (we try and
> expand svaddv as an svbic).
> 
> So one options would be that for LTO we instantiate both sets of tree nodes.
> Then, when expanding a tree node that came from LTO, we dispatch on a flag
> in the tree node (essentially just whether it came from C++ or not) to
> determine which set of functions to use. Seems a bit messy though.
> 
> @Richard: does that sound at all sane? Any ideas for a better approach?
I think we should just make the non-C++ codes line up with the C++ ones
by pushing a dummy entry onto registered_functions (but not creating or
registering a decl for it).

[Bug target/99216] ICE in aarch64_sve::function_expander::expand() with LTO

[acoplan at gcc dot gnu.org]

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

--- Comment #6 from Alex Coplan <acoplan at gcc dot gnu.org> ---
Ok, I'll have a go, thanks.

[Bug target/99216] ICE in aarch64_sve::function_expander::expand() with LTO

[cvs-commit at gcc dot gnu.org]

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

--- Comment #7 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Alex Coplan <acoplan@gcc.gnu.org>:

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

commit r11-7890-ge4005cf8717abe8c949f840c707e02e6c394c2e7
Author: Alex Coplan <alex.coplan@arm.com>
Date:   Mon Mar 29 12:18:19 2021 +0100

    aarch64: Fix SVE ACLE builtins with LTO [PR99216]

    As discussed in the PR, we currently have two different numbering
    schemes for SVE builtins: one for C, and one for C++. This is
    problematic for LTO, where we end up getting confused about which
    intrinsic we're talking about. This patch inserts placeholders into the
    registered_functions vector to ensure that there is a consistent
    numbering scheme for both C and C++.

    We use integer_zero_node as a placeholder node instead of building a
    function decl. This is safe because the node is only returned by the
    TARGET_BUILTIN_DECL hook, which (on AArch64) is only used for validation
    when builtin decls are streamed into lto1.

    gcc/ChangeLog:

            PR target/99216
            * config/aarch64/aarch64-sve-builtins.cc
            (function_builder::add_function): Add placeholder_p argument, use
            placeholder decls if this is set.
            (function_builder::add_unique_function): Instead of conditionally
adding
            direct overloads, unconditionally add either a direct overload or a
            placeholder.
            (function_builder::add_overloaded_function): Set placeholder_p if
we're
            using C++ overloads. Use the obstack for string storage instead
            of relying on the tree nodes.
            (function_builder::add_overloaded_functions): Don't return early
for
            m_direct_overloads: we need to add placeholders.
            * config/aarch64/aarch64-sve-builtins.h
            (function_builder::add_function): Add placeholder_p argument.

    gcc/testsuite/ChangeLog:

            PR target/99216
            * g++.target/aarch64/sve/pr99216.C: New test.

[Bug target/99216] ICE in aarch64_sve::function_expander::expand() with LTO

[acoplan at gcc dot gnu.org]

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

--- Comment #8 from Alex Coplan <acoplan at gcc dot gnu.org> ---
Fixed on trunk. Needs backporting to GCC 10 together with bump to
lto-streamer.h:LTO_minor_version.

[Bug target/99216] ICE in aarch64_sve::function_expander::expand() with LTO

[rsandifo at gcc dot gnu.org]

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

--- Comment #9 from rsandifo at gcc dot gnu.org <rsandifo at gcc dot gnu.org> ---
*** Bug 99252 has been marked as a duplicate of this bug. ***

[Bug target/99216] ICE in aarch64_sve::function_expander::expand() with LTO

[rguenth at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|10.3                        |10.4

--- Comment #10 from Richard Biener <rguenth at gcc dot gnu.org> ---
GCC 10.3 is being released, retargeting bugs to GCC 10.4.

[Bug target/99216] ICE in aarch64_sve::function_expander::expand() with LTO

[cvs-commit at gcc dot gnu.org]

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

--- Comment #11 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The releases/gcc-10 branch has been updated by Alex Coplan
<acoplan@gcc.gnu.org>:

https://gcc.gnu.org/g:34a9bc1f95027eea1560369765b8b2b5722b6779

commit r10-9747-g34a9bc1f95027eea1560369765b8b2b5722b6779
Author: Alex Coplan <alex.coplan@arm.com>
Date:   Thu Apr 22 14:37:12 2021 +0100

    aarch64: Fix SVE ACLE builtins with LTO [PR99216]

    This is a GCC 10 backport of the fix for PR99216
    (e4005cf8717abe8c949f840c707e02e6c394c2e7). The only change w.r.t the
    original patch is a bump of lto-streamer.h:LTO_minor_version.

    As discussed in the PR, we currently have two different numbering
    schemes for SVE builtins: one for C, and one for C++. This is
    problematic for LTO, where we end up getting confused about which
    intrinsic we're talking about. This patch inserts placeholders into the
    registered_functions vector to ensure that there is a consistent
    numbering scheme for both C and C++.

    This version uses integer_zero_node as a placeholder node instead of
    building a function decl. This is safe because the node is only returned
    by the TARGET_BUILTIN_DECL hook, which (on AArch64) is only used for
    validation when builtin decls are streamed into lto1.

    gcc/ChangeLog:

            PR target/99216
            * config/aarch64/aarch64-sve-builtins.cc
            (function_builder::add_function): Add placeholder_p argument, use
            placeholder decls if this is set.
            (function_builder::add_unique_function): Instead of conditionally
adding
            direct overloads, unconditionally add either a direct overload or a
            placeholder.
            (function_builder::add_overloaded_function): Set placeholder_p if
we're
            using C++ overloads. Use the obstack for string storage instead
            of relying on the tree nodes.
            (function_builder::add_overloaded_functions): Don't return early
for
            m_direct_overloads: we need to add placeholders.
            * config/aarch64/aarch64-sve-builtins.h
            (function_builder::add_function): Add placeholder_p argument.
            * lto-streamer.h (LTO_minor_version): Bump.

    gcc/testsuite/ChangeLog:

            PR target/99216
            * g++.target/aarch64/sve/pr99216.C: New test.

[Bug target/99216] ICE in aarch64_sve::function_expander::expand() with LTO

[acoplan at gcc dot gnu.org]

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

Alex Coplan <acoplan at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
         Resolution|---                         |FIXED
             Status|ASSIGNED                    |RESOLVED

--- Comment #12 from Alex Coplan <acoplan at gcc dot gnu.org> ---
Fixed for GCC 10.4 so fixed everywhere.