[Bug target/99540] New: ICE: Segmentation fault in aarch64_add_offset

[Bug target/99540] New: ICE: Segmentation fault in aarch64_add_offset

[asolokha at gmx dot com]

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

            Bug ID: 99540
           Summary: ICE: Segmentation fault in aarch64_add_offset
           Product: gcc
           Version: 11.0
            Status: UNCONFIRMED
          Keywords: ice-on-valid-code
          Severity: normal
          Priority: P3
         Component: target
          Assignee: unassigned at gcc dot gnu.org
          Reporter: asolokha at gmx dot com
  Target Milestone: ---
            Target: aarch64-linux-gnu

gcc-11.0.1-alpha20210307 snapshot (g:a18ebd6c439227b048a91fbfa66f5983f884c157)
ICEs when compiling the following testcase w/ -mcpu=neoverse-n2 -O3
-ffloat-store -ftrapv:

float *MSalign2m2m_rec_initverticalw, *MSalign2m2m_rec_currentw;

void
match_ribosum (int MSalign2m2m_rec_i, int MSalign2m2m_rec_lgth1,
               int MSalign2m2m_rec_lgth2)
{
  float **WMMTX;

  while (MSalign2m2m_rec_i < 1)
    WMMTX[MSalign2m2m_rec_i++][0] = MSalign2m2m_rec_initverticalw[0];

  while (MSalign2m2m_rec_i < MSalign2m2m_rec_lgth1)
    MSalign2m2m_rec_initverticalw[MSalign2m2m_rec_i++] += 0.1;

  while (MSalign2m2m_rec_i < MSalign2m2m_rec_lgth2)
    MSalign2m2m_rec_currentw[MSalign2m2m_rec_i++] += 0.1;
}

% aarch64-linux-gnu-gcc-11.0.1 -mcpu=neoverse-n2 -O3 -ffloat-store -ftrapv -c
i4sksvj6.c
during RTL pass: expand
i4sksvj6.c: In function 'match_ribosum':
i4sksvj6.c:4:1: internal compiler error: Segmentation fault
    4 | match_ribosum (int MSalign2m2m_rec_i, int MSalign2m2m_rec_lgth1,
      | ^~~~~~~~~~~~~
0xedb516 crash_signal
       
/var/tmp/portage/cross-aarch64-linux-gnu/gcc-11.0.1_alpha20210307/work/gcc-11-20210307/gcc/toplev.c:327
0xb193ed force_reg(machine_mode, rtx_def*)
       
/var/tmp/portage/cross-aarch64-linux-gnu/gcc-11.0.1_alpha20210307/work/gcc-11-20210307/gcc/explow.c:671
0x127a3ed aarch64_add_offset
       
/var/tmp/portage/cross-aarch64-linux-gnu/gcc-11.0.1_alpha20210307/work/gcc-11-20210307/gcc/config/aarch64/aarch64.c:4677
0x1293eea aarch64_expand_mov_immediate(rtx_def*, rtx_def*)
       
/var/tmp/portage/cross-aarch64-linux-gnu/gcc-11.0.1_alpha20210307/work/gcc-11-20210307/gcc/config/aarch64/aarch64.c:5281
0x167c29a gen_movdi(rtx_def*, rtx_def*)
       
/var/tmp/portage/cross-aarch64-linux-gnu/gcc-11.0.1_alpha20210307/work/gcc-11-20210307/gcc/config/aarch64/aarch64.md:1249
0xb41ec8 rtx_insn* insn_gen_fn::operator()<rtx_def*, rtx_def*>(rtx_def*,
rtx_def*) const
       
/var/tmp/portage/cross-aarch64-linux-gnu/gcc-11.0.1_alpha20210307/work/gcc-11-20210307/gcc/recog.h:407
0xb41ec8 emit_move_ccmode
       
/var/tmp/portage/cross-aarch64-linux-gnu/gcc-11.0.1_alpha20210307/work/gcc-11-20210307/gcc/expr.c:3640
0xb41ec8 emit_move_insn_1(rtx_def*, rtx_def*)
       
/var/tmp/portage/cross-aarch64-linux-gnu/gcc-11.0.1_alpha20210307/work/gcc-11-20210307/gcc/expr.c:3785
0xb4232e emit_move_insn(rtx_def*, rtx_def*)
       
/var/tmp/portage/cross-aarch64-linux-gnu/gcc-11.0.1_alpha20210307/work/gcc-11-20210307/gcc/expr.c:3936
0xb193ae copy_to_mode_reg(machine_mode, rtx_def*)
       
/var/tmp/portage/cross-aarch64-linux-gnu/gcc-11.0.1_alpha20210307/work/gcc-11-20210307/gcc/explow.c:653
0xdb20e4 maybe_legitimize_operand
       
/var/tmp/portage/cross-aarch64-linux-gnu/gcc-11.0.1_alpha20210307/work/gcc-11-20210307/gcc/optabs.c:7626
0xdb20e4 maybe_legitimize_operands(insn_code, unsigned int, unsigned int,
expand_operand*)
       
/var/tmp/portage/cross-aarch64-linux-gnu/gcc-11.0.1_alpha20210307/work/gcc-11-20210307/gcc/optabs.c:7758
0xdb23e4 maybe_gen_insn(insn_code, unsigned int, expand_operand*)
       
/var/tmp/portage/cross-aarch64-linux-gnu/gcc-11.0.1_alpha20210307/work/gcc-11-20210307/gcc/optabs.c:7777
0xdba917 expand_binop_directly
       
/var/tmp/portage/cross-aarch64-linux-gnu/gcc-11.0.1_alpha20210307/work/gcc-11-20210307/gcc/optabs.c:1409
0xdb6bc5 expand_binop(machine_mode, optab_tag, rtx_def*, rtx_def*, rtx_def*,
int, optab_methods)
       
/var/tmp/portage/cross-aarch64-linux-gnu/gcc-11.0.1_alpha20210307/work/gcc-11-20210307/gcc/optabs.c:1496
0xb479b8 force_operand(rtx_def*, rtx_def*)
       
/var/tmp/portage/cross-aarch64-linux-gnu/gcc-11.0.1_alpha20210307/work/gcc-11-20210307/gcc/expr.c:7753
0xb193ca copy_to_mode_reg(machine_mode, rtx_def*)
       
/var/tmp/portage/cross-aarch64-linux-gnu/gcc-11.0.1_alpha20210307/work/gcc-11-20210307/gcc/explow.c:649
0xdb070f maybe_legitimize_operand_same_code
       
/var/tmp/portage/cross-aarch64-linux-gnu/gcc-11.0.1_alpha20210307/work/gcc-11-20210307/gcc/optabs.c:7575
0xdb070f maybe_legitimize_operand_same_code
       
/var/tmp/portage/cross-aarch64-linux-gnu/gcc-11.0.1_alpha20210307/work/gcc-11-20210307/gcc/optabs.c:7550
0xdb20d4 maybe_legitimize_operand
       
/var/tmp/portage/cross-aarch64-linux-gnu/gcc-11.0.1_alpha20210307/work/gcc-11-20210307/gcc/optabs.c:7623

[Bug target/99540] [10/11 Regression] ICE: Segmentation fault in aarch64_add_offset

[ktkachov at gcc dot gnu.org]

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

ktkachov at gcc dot gnu.org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|---                         |10.3
      Known to work|                            |9.3.1
             Status|UNCONFIRMED                 |NEW
   Last reconfirmed|                            |2021-03-11
     Ever confirmed|0                           |1
                 CC|                            |ktkachov at gcc dot gnu.org
      Known to fail|                            |10.2.1, 11.0
            Summary|ICE: Segmentation fault in  |[10/11 Regression] ICE:
                   |aarch64_add_offset          |Segmentation fault in
                   |                            |aarch64_add_offset

--- Comment #1 from ktkachov at gcc dot gnu.org ---
Confirmed. ICEs with -march=armv8.2-a+sve -O3 -ffloat-store -ftrapv

[Bug target/99540] [10/11 Regression] ICE: Segmentation fault in aarch64_add_offset

[acoplan at gcc dot gnu.org]

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

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

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

--- Comment #2 from Alex Coplan <acoplan at gcc dot gnu.org> ---
So aarch64_add_offset has:

/* Base the factor on LOW_BIT if we can calculate LOW_BIT
   directly, since that should increase the chances of being
   able to use a shift and add sequence.  If LOW_BIT itself
   is out of range, just use CNTD.  */
if (low_bit <= 16 * 8)
  factor /= low_bit;
else
  low_bit = 1;

val = gen_int_mode (poly_int64 (low_bit * 2, low_bit * 2), mode);
val = aarch64_force_temporary (mode, temp1, val);

if (can_create_pseudo_p ())
  {
    rtx coeff1 = gen_int_mode (factor, mode);
    val = expand_mult (mode, val, coeff1, NULL_RTX, false, true);
  }

and it's this call to expand_mult that fails. We're asking for a signed
multiplication (unsignedp=false), we're not allowing libcalls
(no_libcall=true), and we have -ftrapv. It appears that the expand code can't
handle this (we would normally use a libcall here on aarch64).

So I guess one option might be to punt to the else case if the expand_mult
fails.

An interesting question is whether we even want a trapping multiplication here?
Is -ftrapv only meant to trap for multiplications that the user wrote or also
those generated by the compiler?

[Bug target/99540] [10/11 Regression] ICE: Segmentation fault in aarch64_add_offset

[acoplan at gcc dot gnu.org]

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

--- Comment #3 from Alex Coplan <acoplan at gcc dot gnu.org> ---
*** Bug 99554 has been marked as a duplicate of this bug. ***

[Bug target/99540] [10/11 Regression] ICE: Segmentation fault in aarch64_add_offset

[jakub at gcc dot gnu.org]

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

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

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

--- Comment #4 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Given that Richard's change was just an optimization, perhaps we can just do:
--- gcc/config/aarch64/aarch64.c.jj     2021-03-19 10:14:18.302936161 +0100
+++ gcc/config/aarch64/aarch64.c        2021-03-19 12:58:32.454131746 +0100
@@ -4636,7 +4636,7 @@ aarch64_add_offset (scalar_int_mode mode
          val = gen_int_mode (poly_int64 (low_bit * 2, low_bit * 2), mode);
          val = aarch64_force_temporary (mode, temp1, val);

-         if (can_create_pseudo_p ())
+         if (can_create_pseudo_p () && !flag_trapv)
            {
              rtx coeff1 = gen_int_mode (factor, mode);
              val = expand_mult (mode, val, coeff1, NULL_RTX, false, true);


Or

--- gcc/config/aarch64/aarch64.c.jj     2021-03-19 10:14:18.302936161 +0100
+++ gcc/config/aarch64/aarch64.c        2021-03-19 13:03:07.109102334 +0100
@@ -4639,7 +4639,10 @@ aarch64_add_offset (scalar_int_mode mode
          if (can_create_pseudo_p ())
            {
              rtx coeff1 = gen_int_mode (factor, mode);
+             bool saved_trapv = flag_trapv;
+             flag_trapv = false;
              val = expand_mult (mode, val, coeff1, NULL_RTX, false, true);
+             flag_trapv = saved_trapv;
            }
          else
            {

[Bug target/99540] [10/11 Regression] ICE: Segmentation fault in aarch64_add_offset

[jakub at gcc dot gnu.org]

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

--- Comment #5 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Ideally flag_trapv should be reimplemented on top of __builtin_*_overflow, and
do that lowering early rather than at expansion, but that is too late for GCC
11.

[Bug target/99540] [10/11 Regression] ICE: Segmentation fault in aarch64_add_offset

[jakub at gcc dot gnu.org]

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

--- Comment #6 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
As a fix maybe even better
--- gcc/config/aarch64/aarch64.c.jj     2021-03-19 10:14:18.302936161 +0100
+++ gcc/config/aarch64/aarch64.c        2021-03-19 13:33:35.367973092 +0100
@@ -4639,7 +4639,7 @@ aarch64_add_offset (scalar_int_mode mode
          if (can_create_pseudo_p ())
            {
              rtx coeff1 = gen_int_mode (factor, mode);
-             val = expand_mult (mode, val, coeff1, NULL_RTX, false, true);
+             val = expand_mult (mode, val, coeff1, NULL_RTX, true, true);
            }
          else
            {

I mean, for 2's complement target (GCC doesn't support any other), the
difference between non-widening/highpart signed and unsigned multiplication is
just in that flag_trapv case, for 64-bit x 64-bit -> 64-bit multiplication the
result bits are identical whether it is signed or unsigned and that is the
reason why we only have MULT in RTL and not SMULT and UMULT.

[Bug target/99540] [10/11 Regression] ICE: Segmentation fault in aarch64_add_offset

[jakub at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Priority|P3                          |P2

[Bug target/99540] [10/11 Regression] ICE: Segmentation fault in aarch64_add_offset

[rsandifo at gcc dot gnu.org]

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

rsandifo at gcc dot gnu.org <rsandifo at gcc dot gnu.org> changed:

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

--- Comment #7 from rsandifo at gcc dot gnu.org <rsandifo at gcc dot gnu.org> ---
(In reply to Jakub Jelinek from comment #6)
> As a fix maybe even better
> --- gcc/config/aarch64/aarch64.c.jj	2021-03-19 10:14:18.302936161 +0100
> +++ gcc/config/aarch64/aarch64.c	2021-03-19 13:33:35.367973092 +0100
> @@ -4639,7 +4639,7 @@ aarch64_add_offset (scalar_int_mode mode
>  	  if (can_create_pseudo_p ())
>  	    {
>  	      rtx coeff1 = gen_int_mode (factor, mode);
> -	      val = expand_mult (mode, val, coeff1, NULL_RTX, false, true);
> +	      val = expand_mult (mode, val, coeff1, NULL_RTX, true, true);
>  	    }
>  	  else
>  	    {
> 
> I mean, for 2's complement target (GCC doesn't support any other), the
> difference between non-widening/highpart signed and unsigned multiplication
> is just in that flag_trapv case, for 64-bit x 64-bit -> 64-bit
> multiplication the result bits are identical whether it is signed or
> unsigned and that is the reason why we only have MULT in RTL and not SMULT
> and UMULT.
Yeah, this looks good to me, thanks.  In this context (coming from
rtl rather than trees) there's really no sign to speak of, so the
choice of argument was probably pretty arbitrary.  I didn't think
about the effect on flag_trapv.

I'll take the bug to test that and add a testcase.

[Bug target/99540] [10/11 Regression] ICE: Segmentation fault in aarch64_add_offset

[cvs-commit at gcc dot gnu.org]

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

--- Comment #8 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Richard Sandiford <rsandifo@gcc.gnu.org>:

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

commit r11-7787-gd7cea7ceff9a2be7436108030c598628c51fba0f
Author: Richard Sandiford <richard.sandiford@arm.com>
Date:   Tue Mar 23 14:02:03 2021 +0000

    aarch64: Make aarch64_add_offset work with -ftrapv [PR99540]

    aarch64_add_offset uses expand_mult to multiply the SVE VL by an
    out-of-range constant.  expand_mult takes an argument to indicate
    whether the multiplication is signed or unsigned, but in this
    context the multiplication is effectively signless and so the
    choice seemed arbitrary.

    However, one of the things that the signedness input does is
    indicate whether signed overflow should be trapped for -ftrapv.
    We don't want that here, so we must treat the multiplication
    as unsigned.

    gcc/
    2021-03-23  Jakub Jelinek  <jakub@redhat.com>

            PR target/99540
            * config/aarch64/aarch64.c (aarch64_add_offset): Tell
            expand_mult to perform an unsigned rather than a signed
            multiplication.

    gcc/testsuite/
    2021-03-23  Richard Sandiford  <richard.sandiford@arm.com>

            PR target/99540
            * gcc.dg/vect/pr99540.c: New test.

[Bug target/99540] [10 Regression] ICE: Segmentation fault in aarch64_add_offset

[cvs-commit at gcc dot gnu.org]

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

--- Comment #9 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The releases/gcc-10 branch has been updated by Richard Sandiford
<rsandifo@gcc.gnu.org>:

https://gcc.gnu.org/g:7e1c6d7a53a64fdca23b2299b43f57f7a1dece74

commit r10-9528-g7e1c6d7a53a64fdca23b2299b43f57f7a1dece74
Author: Richard Sandiford <richard.sandiford@arm.com>
Date:   Wed Mar 24 19:18:02 2021 +0000

    aarch64: Make aarch64_add_offset work with -ftrapv [PR99540]

    aarch64_add_offset uses expand_mult to multiply the SVE VL by an
    out-of-range constant.  expand_mult takes an argument to indicate
    whether the multiplication is signed or unsigned, but in this
    context the multiplication is effectively signless and so the
    choice seemed arbitrary.

    However, one of the things that the signedness input does is
    indicate whether signed overflow should be trapped for -ftrapv.
    We don't want that here, so we must treat the multiplication
    as unsigned.

    gcc/
    2021-03-24  Jakub Jelinek  <jakub@redhat.com>

            PR target/99540
            * config/aarch64/aarch64.c (aarch64_add_offset): Tell
            expand_mult to perform an unsigned rather than a signed
            multiplication.

    gcc/testsuite/
    2021-03-24  Richard Sandiford  <richard.sandiford@arm.com>

            PR target/99540
            * gcc.dg/vect/pr99540.c: New test.

[Bug target/99540] [10 Regression] ICE: Segmentation fault in aarch64_add_offset

[rsandifo at gcc dot gnu.org]

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

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

[Bug target/99540] [10 Regression] ICE: Segmentation fault in aarch64_add_offset

[rguenth at gcc dot gnu.org]

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

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

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

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

[Bug target/99540] [10 Regression] ICE: Segmentation fault in aarch64_add_offset

[acoplan at gcc dot gnu.org]

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

--- Comment #12 from Alex Coplan <acoplan at gcc dot gnu.org> ---
Looks like this can be closed?

[Bug target/99540] [10 Regression] ICE: Segmentation fault in aarch64_add_offset

[rsandifo at gcc dot gnu.org]

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

rsandifo at gcc dot gnu.org <rsandifo at gcc dot gnu.org> changed:

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

--- Comment #13 from rsandifo at gcc dot gnu.org <rsandifo at gcc dot gnu.org> ---
I think it should be backported back to GCC 8.

[Bug target/99540] ICE: Segmentation fault in aarch64_add_offset

[rsandifo at gcc dot gnu.org]

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

rsandifo at gcc dot gnu.org <rsandifo at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
         Resolution|---                         |FIXED
             Status|ASSIGNED                    |RESOLVED
            Summary|[8/9 Backport] ICE:         |ICE: Segmentation fault in
                   |Segmentation fault in       |aarch64_add_offset
                   |aarch64_add_offset          |

--- Comment #14 from rsandifo at gcc dot gnu.org <rsandifo at gcc dot gnu.org> ---
My bad.  The expand_mult call didn't exist before GCC 10, so all is well.