[Bug tree-optimization/96974] New: [10/11 Regression] ICE in vect_get_vector_types_for_stmt compiling for SVE

[Bug tree-optimization/96974] New: [10/11 Regression] ICE in vect_get_vector_types_for_stmt compiling for SVE

[ktkachov at gcc dot gnu.org]

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

            Bug ID: 96974
           Summary: [10/11 Regression] ICE in
                    vect_get_vector_types_for_stmt compiling for SVE
           Product: gcc
           Version: unknown
            Status: UNCONFIRMED
          Keywords: ice-on-valid-code
          Severity: normal
          Priority: P3
         Component: tree-optimization
          Assignee: unassigned at gcc dot gnu.org
          Reporter: ktkachov at gcc dot gnu.org
  Target Milestone: ---
            Target: aarch64*-*-*

The reduced testcase:

float a;

int
b ()
{ return __builtin_lrintf(a); }

struct c {
  float d;
  c() {
    for (int e = 0; e < 9; e++)
      coeffs[e] = d ? b() : 0;
  }
  int coeffs[10];
} f;

ICEs with g++ -Ofast -march=armv8.2-a+sve

during GIMPLE pass: slp
foo.cc: In function '(static initializers for foo.cc)':
foo.cc:14:4: internal compiler error: in vect_get_vector_types_for_stmt, at
tree-vect-stmts.c:12055
   14 | } f;
      |    ^
0x126e44d vect_get_vector_types_for_stmt(vec_info*, _stmt_vec_info*,
tree_node**, tree_node**, unsigned int)
        $SRC/gcc/tree-vect-stmts.c:12054
0x12a7b9a vect_build_slp_tree_1
        $SRC/gcc/tree-vect-slp.c:801
0x12a7b9a vect_build_slp_tree_2
        $SRC/gcc/tree-vect-slp.c:1284
0x12a642b vect_build_slp_tree
        $SRC/gcc/tree-vect-slp.c:1195
0x12a8a30 vect_build_slp_tree_2
        $SRC/gcc/tree-vect-slp.c:1413
0x12a642b vect_build_slp_tree
        $SRC/gcc/tree-vect-slp.c:1195
0x12a8a30 vect_build_slp_tree_2
        $SRC/gcc/tree-vect-slp.c:1413
0x12a642b vect_build_slp_tree
        $SRC/gcc/tree-vect-slp.c:1195
0x12ab23e vect_analyze_slp_instance
        $SRC/gcc/tree-vect-slp.c:2210
0x12aea06 vect_analyze_slp(vec_info*, unsigned int)
        $SRC/gcc/tree-vect-slp.c:2418
0x12b0fc3 vect_slp_analyze_bb_1
        $SRC/gcc/tree-vect-slp.c:3388
0x12b0fc3 vect_slp_region
        $SRC/gcc/tree-vect-slp.c:3498
0x12b0fc3 vect_slp_bb(basic_block_def*)
        $SRC/gcc/tree-vect-slp.c:3628
0x12b4a70 execute
        $SRC/gcc/tree-vectorizer.c:1429
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.

[Bug tree-optimization/96974] [10/11 Regression] ICE in vect_get_vector_types_for_stmt compiling for SVE

[ktkachov at gcc dot gnu.org]

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

ktkachov at gcc dot gnu.org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
      Known to fail|                            |10.1.1, 11.0
      Known to work|                            |9.3.1
   Target Milestone|---                         |10.4

[Bug tree-optimization/96974] [10/11 Regression] ICE in vect_get_vector_types_for_stmt compiling for SVE

[rguenth at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|10.4                        |10.3
            Version|unknown                     |10.1.1

[Bug tree-optimization/96974] [10/11 Regression] ICE in vect_get_vector_types_for_stmt compiling for SVE

[stammark at gcc dot gnu.org]

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

Stam Markianos-Wright <stammark at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
               Host|                            |x86_64-linux-gnu
                 CC|                            |stammark at gcc dot gnu.org
   Last reconfirmed|                            |2020-10-15

--- Comment #1 from Stam Markianos-Wright <stammark at gcc dot gnu.org> ---
Also confirmed on today's trunk aarch64-none-elf

Seems to relate to use of that specific type of builtin or type of builtins (I
tested some other math builtins and they were ok, but e.g. __builtin_llrintf
produced the same ICE), and only inside the `d ? b() : 0` condition (i.e.
changing that `coeffs[e] = b()` makes the ICE go away)

[Bug tree-optimization/96974] [10/11 Regression] ICE in vect_get_vector_types_for_stmt compiling for SVE

[ktkachov at gcc dot gnu.org]

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

ktkachov at gcc dot gnu.org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |NEW
           Priority|P3                          |P2
   Last reconfirmed|2020-10-15 00:00:00         |2021-01-05
     Ever confirmed|0                           |1

--- Comment #2 from ktkachov at gcc dot gnu.org ---
This has gone latent on GCC 11 trunk with
https://gcc.gnu.org/git/gitweb.cgi?p=gcc.git;h=f5e18dd9c7dacc9671044fc669bd5c1b26b6bdba
and still appears on the GCC 10 branch

[Bug tree-optimization/96974] [10/11 Regression] ICE in vect_get_vector_types_for_stmt compiling for SVE

[stammark at gcc dot gnu.org]

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

--- Comment #3 from Stam Markianos-Wright <stammark at gcc dot gnu.org> ---
Just started looking at this. I've narrowed it as the bug appearing with commit
9b75f56d4b7951c60a656396dddd4a65787b95bc.

I have yet to fire this up in gdb to see what's happening, but one test I did
do was to try commenting out the assert that is causing the ICE and it then ran
to completion. 

So one _total speculation_ would be that with these latest changes that enable
groups of different sizes, this condition in the assert is now too strict:


multiple_p (TYPE_VECTOR_SUBPARTS (nunits_vectype),
            TYPE_VECTOR_SUBPARTS (*stmt_vectype_out)))

[Bug tree-optimization/96974] [10/11 Regression] ICE in vect_get_vector_types_for_stmt compiling for SVE

[rsandifo at gcc dot gnu.org]

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

--- Comment #4 from rsandifo at gcc dot gnu.org <rsandifo at gcc dot gnu.org> ---
(In reply to Stam Markianos-Wright from comment #3)
> Just started looking at this. I've narrowed it as the bug appearing with
> commit 9b75f56d4b7951c60a656396dddd4a65787b95bc.
> 
> I have yet to fire this up in gdb to see what's happening, but one test I
> did do was to try commenting out the assert that is causing the ICE and it
> then ran to completion. 
> 
> So one _total speculation_ would be that with these latest changes that
> enable groups of different sizes, this condition in the assert is now too
> strict:
> 
> 
> multiple_p (TYPE_VECTOR_SUBPARTS (nunits_vectype),
> 	    TYPE_VECTOR_SUBPARTS (*stmt_vectype_out)))
What are nunits_vectype and *stmt_vectype_out at the point
that the assert fails?

[Bug tree-optimization/96974] [10/11 Regression] ICE in vect_get_vector_types_for_stmt compiling for SVE

[stammark at gcc dot gnu.org]

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

--- Comment #5 from Stam Markianos-Wright <stammark at gcc dot gnu.org> ---
(In reply to rsandifo@gcc.gnu.org from comment #4)
> (In reply to Stam Markianos-Wright from comment #3)
> > Just started looking at this. I've narrowed it as the bug appearing with
> > commit 9b75f56d4b7951c60a656396dddd4a65787b95bc.
> > 
> > I have yet to fire this up in gdb to see what's happening, but one test I
> > did do was to try commenting out the assert that is causing the ICE and it
> > then ran to completion. 
> > 
> > So one _total speculation_ would be that with these latest changes that
> > enable groups of different sizes, this condition in the assert is now too
> > strict:
> > 
> > 
> > multiple_p (TYPE_VECTOR_SUBPARTS (nunits_vectype),
> > 	    TYPE_VECTOR_SUBPARTS (*stmt_vectype_out)))
> What are nunits_vectype and *stmt_vectype_out at the point
> that the assert fails?

Hmm so looking at this on commit 9b75f56d4b7951c60a656396dddd4a65787b95bc,
we have:



nunits_vectype is a: vector(SUBPARTS {coeffs = {4, 0}}) float

*stmt_vectype_out is a: vector(SUBPARTS {coeffs = {8, 0}}) long int

In this case we are checking multiple_p (4, 8) == false
(and also group_size == 9 here which is expected)



Before the commit we'd get here with:



nunits_vectype is a: vector(SUBPARTS {coeffs = {4, 0}}) float

*stmt_vectype_out is a: vector(SUBPARTS {coeffs = {2, 0}}) long int

And here we were checking multiple_p (4, 2) == true



I'm tempted to try and add a reverse:

        || multiple_p (*stmt_vectype_out, nunits_vectype)

And then regtest, but I probably need to do more reading around to figure out
what we really should be expecting each case!

[Bug tree-optimization/96974] [10/11 Regression] ICE in vect_get_vector_types_for_stmt compiling for SVE

[rsandifo at gcc dot gnu.org]

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

--- Comment #6 from rsandifo at gcc dot gnu.org <rsandifo at gcc dot gnu.org> ---
(In reply to Stam Markianos-Wright from comment #5)
> I'm tempted to try and add a reverse:
> 
> 	|| multiple_p (*stmt_vectype_out, nunits_vectype)
> 
> And then regtest, but I probably need to do more reading around to figure out
> what we really should be expecting each case!
I don't think that's right.  If nunits_vectype is not a multiple
of stmt_vectype then the stmt_vectype contains (or might contain)
unused elements.  The vectoriser isn't set up to work like that:
all operations are currently supposed to be full-vector operations
(possibly predicated, on SVE and AVX).

AFAICT the assert is correct and it's showing up a problem elsewhere.

[Bug tree-optimization/96974] [10/11 Regression] ICE in vect_get_vector_types_for_stmt compiling for SVE

[stammark at gcc dot gnu.org]

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

--- Comment #7 from Stam Markianos-Wright <stammark at gcc dot gnu.org> ---
(In reply to rsandifo@gcc.gnu.org from comment #6)
> (In reply to Stam Markianos-Wright from comment #5)
> > I'm tempted to try and add a reverse:
> > 
> > 	|| multiple_p (*stmt_vectype_out, nunits_vectype)
> > 
> > And then regtest, but I probably need to do more reading around to figure out
> > what we really should be expecting each case!
> I don't think that's right.  If nunits_vectype is not a multiple
> of stmt_vectype then the stmt_vectype contains (or might contain)
> unused elements.  The vectoriser isn't set up to work like that:
> all operations are currently supposed to be full-vector operations
> (possibly predicated, on SVE and AVX).
> 
> AFAICT the assert is correct and it's showing up a problem elsewhere.

Cool, thank you for the info and the confirmation! I will carry on
investigating to try and find the actual problem

[Bug tree-optimization/96974] [10/11 Regression] ICE in vect_get_vector_types_for_stmt compiling for SVE

[stammark at gcc dot gnu.org]

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

--- Comment #8 from Stam Markianos-Wright <stammark at gcc dot gnu.org> ---
I have a liiitle bit more progress here, but I have a question about
vect_get_smallest_scalar_type.

If we look at the comment before the function:

>/* Return the smallest scalar part of STMT_INFO.
>   This is used to determine the vectype of the stmt.  We generally set the
>   vectype according to the type of the result (lhs).  For stmts whose
>   result-type is different than the type of the arguments (e.g., demotion,
>   promotion), vectype will be reset appropriately (later).  Note that we have
>   to visit the smallest datatype in this function, because that determines the
>   VF.  If the smallest datatype in the loop is present only as the rhs of a
>   promotion operation - we'd miss it.

Would this be "smallest datatype in all cases", or is this more like "the
smallest
datatype within the same promotion/demotion chain"?

i.e. how should we react if we detect a smallest datatype on the rhs of "float"
when everything else in the stmt has been in the integer chain (int or,
like in this case, long int)?

>   Such a case, where a variable of this datatype does not appear in the lhs
>   anywhere in the loop, can only occur if it's an invariant: e.g.:
>   'int_x = (int) short_inv', which we'd expect to have been optimized away by
>   invariant motion.  However, we cannot rely on invariant motion to always
>   take invariants out of the loop, and so in the case of promotion we also
>   have to check the rhs.
>   LHS_SIZE_UNIT and RHS_SIZE_UNIT contain the sizes of the corresponding
>   types.  */

I have found that this is why we end up with a smaller number in:
TYPE_VECTOR_SUBPARTS (nunits_vectype) == 4
than in:
TYPE_VECTOR_SUBPARTS (*stmt_vectype_out) == 8

So I'm thinking that either A) We shouldn't allow this, and add in some check
maybe for "GET_MODE_CLASS (x) == GET_MODE_CLASS (y)"
or B) Some of the logic that generates stmt_vectype_out is deficient and it
should also be detecting the existence of a "float" type to get the VF.

[Bug tree-optimization/96974] [10/11 Regression] ICE in vect_get_vector_types_for_stmt compiling for SVE

[stammark at gcc dot gnu.org]

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

Stam Markianos-Wright <stammark at gcc dot gnu.org> changed:

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

[Bug tree-optimization/96974] [10/11 Regression] ICE in vect_get_vector_types_for_stmt compiling for SVE

[rsandifo at gcc dot gnu.org]

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

--- Comment #9 from rsandifo at gcc dot gnu.org <rsandifo at gcc dot gnu.org> ---
(In reply to Stam Markianos-Wright from comment #8)
> I have a liiitle bit more progress here, but I have a question about
> vect_get_smallest_scalar_type.
> 
> If we look at the comment before the function:
> 
> >/* Return the smallest scalar part of STMT_INFO.
> >   This is used to determine the vectype of the stmt.  We generally set the
> >   vectype according to the type of the result (lhs).  For stmts whose
> >   result-type is different than the type of the arguments (e.g., demotion,
> >   promotion), vectype will be reset appropriately (later).  Note that we have
> >   to visit the smallest datatype in this function, because that determines the
> >   VF.  If the smallest datatype in the loop is present only as the rhs of a
> >   promotion operation - we'd miss it.
> 
> Would this be "smallest datatype in all cases", or is this more like "the
> smallest
> datatype within the same promotion/demotion chain"?
> 
> i.e. how should we react if we detect a smallest datatype on the rhs of
> "float"
> when everything else in the stmt has been in the integer chain (int or,
> like in this case, long int)?
Not sure if this is really answering the question (let me know if it's not),
but: for an lhs of int and an rhs of float, the choice doesn't really matter,
since they're the same size.  But for an lhs of long int and an rhs of float,
the smallest vectype has to be taken from the float.

[Bug tree-optimization/96974] [10/11 Regression] ICE in vect_get_vector_types_for_stmt compiling for SVE

[rguenth at gcc dot gnu.org]

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

--- Comment #10 from Richard Biener <rguenth at gcc dot gnu.org> ---
Still reproduces on trunk with -fdisable-tree-fre4.

So the issue is that for the case of 'long int' and 'float'
get_vectype_for_scalar_type when passed 8 as group_size returns
V8DI and V4SF - where eventually V16SF does not exist.

The vinfos vector mode is E_VNx4SFmode.

We're rejecting that because it's

11122         && maybe_ge (TYPE_VECTOR_SUBPARTS (vectype), group_size))

and iterating with get_related_vectype_for_scalar_type arriving at v4sf.

OTOH for long int we start with vector([2,2]) long int and iterate to v8di.

I guess the assert is really misplaced and it should instead try to find
a proper related type based on the nunits vectype?

Or give up when we arrive at such incompatible choices for input/output
vector types.

[Bug tree-optimization/96974] [10/11 Regression] ICE in vect_get_vector_types_for_stmt compiling for SVE

[cvs-commit at gcc dot gnu.org]

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

--- Comment #11 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Stam Markianos-Wright
<stammark@gcc.gnu.org>:

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

commit r11-7830-gaac12084fc07319d5c8232c51dafa4e297bd5415
Author: Stam Markianos-Wright <stam.markianos-wright@arm.com>
Date:   Thu Mar 25 15:29:41 2021 +0000

    tree-optimization/96974 - avoid ICE by replacing assert with standard
failure

    Minor patch to add a graceful exit in the rare case where an invalid
    combination of TYPE_VECTOR_SUBPARTS for nunits_vectype and
    *stmt_vectype_out is reached in vect_get_vector_types_for_stmt.

    This resolves the ICE seen in PR tree-optimization/96974, however the issue
    of correctly handling this rare vectorization combination is left for a
    later patch.

    Bootstrapped and reg-tested on aarch64-linux-gnu.

    2021-03-25  Stam Markianos-Wright  <stam.markianos-wright@arm.com>

    gcc/ChangeLog:

            PR tree-optimization/96974
            * tree-vect-stmts.c (vect_get_vector_types_for_stmt): Replace
assert
            with graceful exit.

    gcc/testsuite/ChangeLog:

            PR tree-optimization/96974
            * g++.target/aarch64/sve/pr96974.C: New test.

[Bug tree-optimization/96974] [10/11 Regression] ICE in vect_get_vector_types_for_stmt compiling for SVE

[cvs-commit at gcc dot gnu.org]

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

--- Comment #12 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The releases/gcc-10 branch has been updated by Stam Markianos-Wright
<stammark@gcc.gnu.org>:

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

commit r10-9545-ga1b4d742f180ff6f1e538e79e590065afe2cce6e
Author: Stam Markianos-Wright <stam.markianos-wright@arm.com>
Date:   Thu Mar 25 15:31:17 2021 +0000

    tree-optimization/96974 - avoid ICE by replacing assert with standard
failure

    Minor patch to add a graceful exit in the rare case where an invalid
    combination of TYPE_VECTOR_SUBPARTS for nunits_vectype and
    *stmt_vectype_out is reached in vect_get_vector_types_for_stmt.

    This resolves the ICE seen in PR tree-optimization/96974, however the issue
    of correctly handling this rare vectorization combination is left for a
    later patch.

    Bootstrapped and reg-tested on aarch64-linux-gnu.

    2021-03-25  Stam Markianos-Wright  <stam.markianos-wright@arm.com>

    gcc/ChangeLog:

            PR tree-optimization/96974
            * tree-vect-stmts.c (vect_get_vector_types_for_stmt): Replace
assert
            with graceful exit.

    gcc/testsuite/ChangeLog:

            PR tree-optimization/96974
            * g++.target/aarch64/sve/pr96974.C: New test.

[Bug tree-optimization/96974] [10/11 Regression] ICE in vect_get_vector_types_for_stmt compiling for SVE

[ktkachov at gcc dot gnu.org]

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

--- Comment #13 from ktkachov at gcc dot gnu.org ---
Fixed now?

[Bug tree-optimization/96974] [10/11 Regression] ICE in vect_get_vector_types_for_stmt compiling for SVE

[rguenth at gcc dot gnu.org]

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

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

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

--- Comment #14 from Richard Biener <rguenth at gcc dot gnu.org> ---
Fixed.

[Bug tree-optimization/96974] [10/11 Regression] ICE in vect_get_vector_types_for_stmt compiling for SVE

[cvs-commit at gcc dot gnu.org]

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

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

https://gcc.gnu.org/g:05de07136a8c288086def19fa7a6ed817e26c6aa

commit r11-7930-g05de07136a8c288086def19fa7a6ed817e26c6aa
Author: Christophe Lyon <christophe.lyon@linaro.org>
Date:   Mon Mar 29 11:36:41 2021 +0000

    testsuite/aarch64: Skip SLP diagnostic under ILP32 (PR target/96974)

    The vectorizer has a very different effect with -mabi=ilp32, and
    doesn't emit the expecte diagnostic, so this patch expects it only
    under lp64.

    2021-03-29  Christophe Lyon  <christophe.lyon@linaro.org>

            gcc/testsuite/
            PR target/96974
            * g++.target/aarch64/sve/pr96974.C: Expect SLP diagnostic only
            under lp64.

[Bug tree-optimization/96974] [10/11 Regression] ICE in vect_get_vector_types_for_stmt compiling for SVE

[cvs-commit at gcc dot gnu.org]

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

--- Comment #16 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The releases/gcc-10 branch has been updated by Christophe Lyon
<clyon@gcc.gnu.org>:

https://gcc.gnu.org/g:285f65a59cebd20d8222c515eb15debd35f9f4a1

commit r10-9643-g285f65a59cebd20d8222c515eb15debd35f9f4a1
Author: Christophe Lyon <christophe.lyon@linaro.org>
Date:   Mon Mar 29 11:36:41 2021 +0000

    testsuite/aarch64: Skip SLP diagnostic under ILP32 (PR target/96974)

    The vectorizer has a very different effect with -mabi=ilp32, and
    doesn't emit the expecte diagnostic, so this patch expects it only
    under lp64.

    2021-03-29  Christophe Lyon  <christophe.lyon@linaro.org>

            gcc/testsuite/
            PR target/96974
            * g++.target/aarch64/sve/pr96974.C: Expect SLP diagnostic only
            under lp64.