[Bug tree-optimization/97405] New: ICE in get_or_alloc_expr_for in code hoisting with SVE intrinsics

[Bug tree-optimization/97405] New: ICE in get_or_alloc_expr_for in code hoisting with SVE intrinsics

[ktkachov at gcc dot gnu.org]

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

            Bug ID: 97405
           Summary: ICE in get_or_alloc_expr_for in code hoisting with SVE
                    intrinsics
           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 following ICEs:
#include "arm_sve.h"

void
a (svuint8x3_t b, unsigned char *p, int c) {
  if (c)
    svst1_u8(svptrue_pat_b8(SV_VL16), p, svget3_u8(b, 1));
  else
    svst1_u8(svwhilelt_b8(6, 6), p, svget3_u8(b, 1));
}

with -O2 -march=armv8.2-a+sve on aarch64 with both GCC 10 and 11 branches

during GIMPLE pass: pre
sveice.c: In function 'a':
sveice.c:4:1: internal compiler error: in get_or_alloc_expr_for, at
tree-ssa-pre.c:1098
    4 | a (svuint8x3_t b, unsigned char *p, int c) {
      | ^
0xf3a0e9 get_or_alloc_expr_for
        $SRC/gcc/tree-ssa-pre.c:1098
0xf3a0e9 find_or_generate_expression
        $SRC/gcc/tree-ssa-pre.c:2693
0xf3aadd create_component_ref_by_pieces_1
        $SRC/gcc/tree-ssa-pre.c:2613
0xf393fc create_component_ref_by_pieces
        $SRC/gcc/tree-ssa-pre.c:2681
0xf393fc create_expression_by_pieces
        $SRC/gcc/tree-ssa-pre.c:2830
0xf3da24 do_hoist_insertion
        $SRC/gcc/tree-ssa-pre.c:3598
0xf3da24 insert
        $SRC/gcc/tree-ssa-pre.c:3685
0xf3da24 execute
        $SRC/gcc/tree-ssa-pre.c:4235
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/97405] ICE in get_or_alloc_expr_for in code hoisting with SVE intrinsics

[ktkachov at gcc dot gnu.org]

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

ktkachov at gcc dot gnu.org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|---                         |10.2
      Known to fail|                            |10.1.1, 11.0

[Bug tree-optimization/97405] ICE in get_or_alloc_expr_for in code hoisting with SVE intrinsics

[rguenth at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Assignee|unassigned at gcc dot gnu.org      |rguenth at gcc dot gnu.org
            Version|unknown                     |10.2.1
   Last reconfirmed|                            |2020-10-14
     Ever confirmed|0                           |1
             Status|UNCONFIRMED                 |ASSIGNED
           Priority|P3                          |P2

--- Comment #1 from Richard Biener <rguenth at gcc dot gnu.org> ---
Confirmed, I'll have a look.

[Bug tree-optimization/97405] ICE in get_or_alloc_expr_for in code hoisting with SVE intrinsics

[rguenth at gcc dot gnu.org]

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

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

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

--- Comment #2 from Richard Biener <rguenth at gcc dot gnu.org> ---
OK, looks like the code expects

POLY_INT_CST [16, 16] /[ex] 16

to be constant folded - looks like poly_int_binop doesn't handle division
nor is there a wi::div_trunc for poly_wide_int.

This is generated here:

Breakpoint 5, copy_reference_ops_from_ref (ref=<array_ref 0x7ffff642fa10>, 
891                 /* But record element size in units of the type alignment. 
*/
892                 temp.op2 = TREE_OPERAND (ref, 3);
893                 temp.align = eltype->type_common.align;
894                 if (! temp.op2)
895                   temp.op2 = size_binop (EXACT_DIV_EXPR, TYPE_SIZE_UNIT
(eltype),
896                                          size_int (TYPE_ALIGN_UNIT
(eltype)));

Hmm, I see that temp.op2 is indeed always needed unless we want to disrupt
valueization.  copy_reference_ops_from_ref also isn't really allowed to fail.

So we absolutely need to be able to fold this EXACT/TRUNC_DIV_EXPR.

Richard?

[Bug tree-optimization/97405] ICE in get_or_alloc_expr_for in code hoisting with SVE intrinsics

[rsandifo at gcc dot gnu.org]

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

--- Comment #3 from rsandifo at gcc dot gnu.org <rsandifo at gcc dot gnu.org> ---
(In reply to Richard Biener from comment #2)
> OK, looks like the code expects
> 
> POLY_INT_CST [16, 16] /[ex] 16
> 
> to be constant folded - looks like poly_int_binop doesn't handle division
> nor is there a wi::div_trunc for poly_wide_int.
> 
> This is generated here:
> 
> Breakpoint 5, copy_reference_ops_from_ref (ref=<array_ref 0x7ffff642fa10>, 
> 891                 /* But record element size in units of the type
> alignment.  */
> 892                 temp.op2 = TREE_OPERAND (ref, 3);
> 893                 temp.align = eltype->type_common.align;
> 894                 if (! temp.op2)
> 895                   temp.op2 = size_binop (EXACT_DIV_EXPR, TYPE_SIZE_UNIT
> (eltype),
> 896                                          size_int (TYPE_ALIGN_UNIT
> (eltype)));
> 
> Hmm, I see that temp.op2 is indeed always needed unless we want to disrupt
> valueization.  copy_reference_ops_from_ref also isn't really allowed to fail.
> 
> So we absolutely need to be able to fold this EXACT/TRUNC_DIV_EXPR.
> 
> Richard?
Yeah, EXACT_DIV_EXPR can't in general be folded at compile time
because it wraps differently.  We could perhaps fold it if
TYPE_OVERFLOW_UNDEFINED is true, but by the letter of the law,
we should issue a warning when doing that.

I guess we want something similar to bd8a2482569900a114d631f9c38353d3a96a1d85
(And it would be nice if we didn't have too many copies of this code. ;-))

[Bug tree-optimization/97405] ICE in get_or_alloc_expr_for in code hoisting with SVE intrinsics

[rguenth at gcc dot gnu.org]

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

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

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

--- Comment #4 from Richard Biener <rguenth at gcc dot gnu.org> ---
(In reply to rsandifo@gcc.gnu.org from comment #3)
> (In reply to Richard Biener from comment #2)
> > OK, looks like the code expects
> > 
> > POLY_INT_CST [16, 16] /[ex] 16
> > 
> > to be constant folded - looks like poly_int_binop doesn't handle division
> > nor is there a wi::div_trunc for poly_wide_int.
> > 
> > This is generated here:
> > 
> > Breakpoint 5, copy_reference_ops_from_ref (ref=<array_ref 0x7ffff642fa10>, 
> > 891                 /* But record element size in units of the type
> > alignment.  */
> > 892                 temp.op2 = TREE_OPERAND (ref, 3);
> > 893                 temp.align = eltype->type_common.align;
> > 894                 if (! temp.op2)
> > 895                   temp.op2 = size_binop (EXACT_DIV_EXPR, TYPE_SIZE_UNIT
> > (eltype),
> > 896                                          size_int (TYPE_ALIGN_UNIT
> > (eltype)));
> > 
> > Hmm, I see that temp.op2 is indeed always needed unless we want to disrupt
> > valueization.  copy_reference_ops_from_ref also isn't really allowed to fail.
> > 
> > So we absolutely need to be able to fold this EXACT/TRUNC_DIV_EXPR.
> > 
> > Richard?
> Yeah, EXACT_DIV_EXPR can't in general be folded at compile time
> because it wraps differently.  We could perhaps fold it if
> TYPE_OVERFLOW_UNDEFINED is true, but by the letter of the law,
> we should issue a warning when doing that.
> 
> I guess we want something similar to bd8a2482569900a114d631f9c38353d3a96a1d85
> (And it would be nice if we didn't have too many copies of this code. ;-))

Ugh.  Maybe revisit, for the Nth time, the scaling by the alignment unit of
the COMPONENT_REF/ARRAY_REF sizes ... RTL expansion needs to recover
alignment guarantees for those in case of variable accesses on strict
alignment targets and the MULT_EXPRs we get from array_ref_element_size
and friends help the various helpers derive an alignment.  The last attempt
isn't that old but Eric shot it down in the end.

[Bug tree-optimization/97405] ICE in get_or_alloc_expr_for in code hoisting with SVE intrinsics

[rguenth at gcc dot gnu.org]

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

--- Comment #5 from Richard Biener <rguenth at gcc dot gnu.org> ---
It was https://gcc.gnu.org/legacy-ml/gcc-patches/2016-02/msg01260.html with
relevant followups from Eric in May.

[Bug tree-optimization/97405] ICE in get_or_alloc_expr_for in code hoisting with SVE intrinsics

[pinskia at gcc dot gnu.org]

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

Andrew Pinski <pinskia at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|10.2                        |---
   Last reconfirmed|2020-10-14 00:00:00         |2022-1-8
      Known to fail|                            |12.0

[Bug tree-optimization/97405] ICE in get_or_alloc_expr_for in code hoisting with SVE intrinsics

[pinskia at gcc dot gnu.org]

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

Andrew Pinski <pinskia at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
      Known to work|                            |15.0

--- Comment #6 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
Fixed on the trunk, I suspect r15-917-gc9842f99042454 fixed it.
That revision is explictly fixing `POLY_INT_CST [16, 16] /[ex] 16` case too.

I think we should just add the testcase and close it as fixed.

[Bug tree-optimization/97405] ICE in get_or_alloc_expr_for in code hoisting with SVE intrinsics

[pinskia at gcc dot gnu.org]

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

Andrew Pinski <pinskia at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           See Also|                            |https://gcc.gnu.org/bugzill
                   |                            |a/show_bug.cgi?id=115214
             Status|NEW                         |ASSIGNED
           Assignee|unassigned at gcc dot gnu.org      |pinskia at gcc dot gnu.org

--- Comment #7 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
(In reply to Andrew Pinski from comment #6)
> Fixed on the trunk, I suspect r15-917-gc9842f99042454 fixed it.
> That revision is explictly fixing `POLY_INT_CST [16, 16] /[ex] 16` case too.
> 
> I think we should just add the testcase and close it as fixed.

yes that fixed it, will commit a testcase in a little bit.

[Bug tree-optimization/97405] ICE in get_or_alloc_expr_for in code hoisting with SVE intrinsics

[cvs-commit at gcc dot gnu.org]

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

--- Comment #8 from GCC Commits <cvs-commit at gcc dot gnu.org> ---
The trunk branch has been updated by Andrew Pinski <pinskia@gcc.gnu.org>:

https://gcc.gnu.org/g:17979deb15d34dd4f036ca93d2977d0fc4d556a7

commit r15-1387-g17979deb15d34dd4f036ca93d2977d0fc4d556a7
Author: Andrew Pinski <quic_apinski@quicinc.com>
Date:   Mon Jun 17 16:45:34 2024 -0700

    aarch64: Add testcase for PR97405

    This aarch64 sve specific code was fixed by r15-917-gc9842f99042454
    which added a riscv specific testcase so adding an aarch64 one to test
    the fix does not regress is a good idea.

    Committed as obvious after testing the testcase for aarch64-linux-gnu.

            PR tree-optimization/97405

    gcc/testsuite/ChangeLog:

            * gcc.target/aarch64/sve/pr97405-1.c: New test.

    Signed-off-by: Andrew Pinski <quic_apinski@quicinc.com>

[Bug tree-optimization/97405] ICE in get_or_alloc_expr_for in code hoisting with SVE intrinsics

[pinskia at gcc dot gnu.org]

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

Andrew Pinski <pinskia at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|ASSIGNED                    |RESOLVED
   Target Milestone|---                         |15.0
         Resolution|---                         |FIXED

--- Comment #9 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
Fixed.