[Bug target/99593] New: [11 Regression] arm MVE ICE when compiling firefox (skia) since r11-6708

[Bug target/99593] New: [11 Regression] arm MVE ICE when compiling firefox (skia) since r11-6708

[jakub at gcc dot gnu.org]

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

            Bug ID: 99593
           Summary: [11 Regression] arm MVE ICE when compiling firefox
                    (skia) since r11-6708
           Product: gcc
           Version: 11.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: ---

typedef __simd64_int16_t e;
typedef __simd64_uint16_t f;
typedef __simd128_int32_t g;
typedef __simd128_float32_t h;
typedef __simd128_uint32_t i;
g j, p;
g k(int l) { return __builtin_neon_vdup_nv4si(l); }
i n(f l) { return (i)__builtin_neon_vmovluv4hi((e)l); }
template <int, typename> struct q;
template <int r, typename aa> q<r, aa> operator<(aa s, q<r, aa> t) {
  return q<r, aa>(s) < t;
}
template <typename ab, typename ac, int r> q<r, ab> ad(const q<r, ac> &);
typedef q<4, int> ae;
template <> class q<4, float> {
public:
  q(h af) : ag(af) {}
  q(float) {}
  static q ah(void *ai) {
    float *l = (float *)ai;
    return __builtin_neon_vld1v4sf(l);
  }
  q operator+(q o) {
    h l = ag, m = o.ag;
    return __builtin_neon_vaddv4sf(l, m);
  }
  q operator*(q) {
    h l = ag, m;
    return __builtin_neon_vmulfv4sf(l, m);
  }
  h ag;
};
template <> class q<4, unsigned short> {
public:
  q(f af) : ag(af) {}
  static q ah(void *ai) {
    unsigned short *l = (unsigned short *)ai;
    return (f)__builtin_neon_vld1v4hi((__builtin_neon_hi *)l);
  }
  void aj() {
    f m = ag;
    __builtin_neon_vst1v4hi(0, (e)m);
  }
  f ag;
};
template <> class q<4, int> {
public:
  q(g af) : ag(af) {}
  q(int u) { ag = k(u); }
  static q ah(void *ai) {
    int *l = (int *)ai;
    return __builtin_neon_vld1v4si(l);
  }
  q operator&(q o) {
    g v = ag & o.ag;
    return v;
  }
  q operator|(q o) {
    g w = ag | o.ag;
    return w;
  }
  q operator^(q) {
    g x = ag ^ p;
    return x;
  }
  q operator>>(int ak) { return ag >> q(ak).ag; }
  q operator<(q) {
    g y, z = j < ag;
    y = (g)z;
    return y;
  }
  g ag;
};
template <> ae ad(const q<4, unsigned short> &al) { return g(n(al.ag)); }
template <> q<4, unsigned short> ad(const ae &al) {
  i l(i(al.ag));
  return (f)__builtin_neon_vmovnv4si((g)l);
}
q<4, float> am(long long an) {
  q ao = q<4, unsigned short>::ah(&an);
  ae ak = ad<int>(ao), ap = ak & 8000, aq = ak ^ ap, ar = 55 < aq, as(aq);
  q at = as & ar;
  ae au = ap | at;
  return q<4, float>::ah(&au);
}
q<4, unsigned short> av(q<4, float> aw) {
  ae ak = ae::ah(&aw), ap = ak & 80000000, aq = ap, ax = 5, as = aq >> 3,
     ay = 6;
  q az = ax & as;
  ae au = ay | az;
  return ad<unsigned short>(au);
}
struct ba {
  typedef int bb;
  static q<4, float> bc(int s) { return am(s); }
};
q<4, float> bd(q<4, float> s) { return s * 0; }
template <typename be> void bf(void *bg, void *al, int bh, int bi) {
  int bj;
  auto bk(static_cast<typename be::bb *>(al) + bh),
      d = static_cast<typename be::bb *>(bg),
      bl = be::bc(static_cast<typename be::bb *>(al)[0]), bm = be::bc(0),
      c = bm;
  for (; bi;) {
    auto a = c, bn = be::bc(static_cast<typename be::bb *>(al)[1]),
         bo = be::bc(1);
    q bp = bn;
    q bq = bp;
    auto b = bq + bo;
    bl = be::bc(static_cast<typename be::bb *>(al)[2]);
    bm = be::bc(bk[2]);
    c = bl + bm;
    q br = a + b;
    auto bs = br;
    q bt = bd(bs);
    av(bt).aj();
    d[0] = bj;
  }
}
int bu;
void bv() { bf<ba>(0, 0, 0, bu); }

reduced from firefox (skia) ICEs with -mtune=generic-armv7-a -mfloat-abi=hard
-mfpu=neon -O2 -std=c++17:
ccLSOUAW.ii: In function ‘void bf(void*, void*, int, int) [with be = ba]’:
ccLSOUAW.ii:119:1: internal compiler error: in neon_output_shift_immediate, at
config/arm/arm.c:12993
  119 | }
      | ^
0x1adad35 neon_output_shift_immediate(char const*, char, rtx_def**,
machine_mode, int, bool)
        ../../gcc/config/arm/arm.c:12993
0x212ff23 output_510
        ../../gcc/config/arm/arm.md:12899
0x111be52 get_insn_template(int, rtx_insn*)
        ../../gcc/final.c:2072
0x111e0c3 final_scan_insn_1
        ../../gcc/final.c:3058
0x111e54b final_scan_insn(rtx_insn*, _IO_FILE*, int, int, int*)
        ../../gcc/final.c:3171
0x111bc83 final_1
        ../../gcc/final.c:2022
0x1121245 rest_of_handle_final
        ../../gcc/final.c:4676
0x11215a4 execute
        ../../gcc/final.c:4754
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.

since r11-6708-gbfab355012ca0f5219da8beb04f2fdaf757d34b7

[Bug target/99593] [11 Regression] arm MVE ICE when compiling firefox (skia) since r11-6708

[jakub at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Priority|P3                          |P1
                 CC|                            |clyon at gcc dot gnu.org,
                   |                            |ktkachov at gcc dot gnu.org
   Target Milestone|---                         |11.0
   Last reconfirmed|                            |2021-03-15
     Ever confirmed|0                           |1
             Status|UNCONFIRMED                 |NEW

--- Comment #1 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
I think the bug is in mismatch between predicates and constraints:
(define_insn "mve_vshlq_<supf><mode>"
  [(set (match_operand:VDQIW 0 "s_register_operand" "=w,w")
        (unspec:VDQIW [(match_operand:VDQIW 1 "s_register_operand" "w,w")
                       (match_operand:VDQIW 2 "imm_lshift_or_reg_neon" "w,Dm")]
         VSHLQ))]
  "ARM_HAVE_<MODE>_ARITH && !TARGET_REALLY_IWMMXT"
  "@
   vshl.<supf>%#<V_sz_elem>\t%<V_reg>0, %<V_reg>1, %<V_reg>2
   * return neon_output_shift_immediate (\"vshl\", 'i', &operands[2],
<MODE>mode, VALID_NEON_QREG_MODE (<MODE>mode), true);"
  [(set_attr "type" "neon_shift_reg<q>, neon_shift_imm<q>")]
)

imm_lshift_or_reg_neon predicate for CONST_VECTOR implies
imm_for_neon_lshift_operand, but the Dm constraint stands for
imm_for_neon_mov_operand.

--- gcc/config/arm/constraints.md.jj    2021-01-04 10:25:44.136173777 +0100
+++ gcc/config/arm/constraints.md       2021-03-15 10:49:28.660795496 +0100
@@ -32,7 +32,7 @@

 ;; The following multi-letter normal constraints have been used:
 ;; in ARM/Thumb-2 state: Da, Db, Dc, Dd, Dn, DN, Dm, Dl, DL, Do, Dv, Dy, Di,
-;;                      Dt, Dp, Dz, Tu, Te
+;;                      Ds, Dt, Dp, Dz, Tu, Te
 ;; in Thumb-1 state: Pa, Pb, Pc, Pd, Pe
 ;; in Thumb-2 state: Ha, Pj, PJ, Ps, Pt, Pu, Pv, Pw, Px, Py, Pz, Rd, Rf, Rb,
Ra,
 ;;                  Rg, Ri
@@ -412,6 +412,14 @@ (define_constraint "Dt"
   (and (match_code "const_double")
        (match_test "TARGET_32BIT && vfp3_const_double_for_fract_bits (op)")))

+(define_constraint "Ds"
+ "@internal
+  In ARM/Thumb-2 state a const_vector which can be used as immediate
+  in vshl instruction."
+ (and (match_code "const_vector")
+      (match_test "TARGET_32BIT
+                  && imm_for_neon_lshift_operand (op, GET_MODE (op))")))
+
 (define_constraint "Dp"
  "@internal
   In ARM/ Thumb2 a const_double which can be used with a vcvt.s32.f32 with
bits operation"
--- gcc/config/arm/vec-common.md.jj     2021-01-29 11:54:15.650661610 +0100
+++ gcc/config/arm/vec-common.md        2021-03-15 10:50:26.570150770 +0100
@@ -299,7 +299,7 @@ (define_expand "movmisalign<mode>"
 (define_insn "mve_vshlq_<supf><mode>"
   [(set (match_operand:VDQIW 0 "s_register_operand" "=w,w")
        (unspec:VDQIW [(match_operand:VDQIW 1 "s_register_operand" "w,w")
-                      (match_operand:VDQIW 2 "imm_lshift_or_reg_neon" "w,Dm")]
+                      (match_operand:VDQIW 2 "imm_lshift_or_reg_neon" "w,Ds")]
         VSHLQ))]
   "ARM_HAVE_<MODE>_ARITH && !TARGET_REALLY_IWMMXT"
   "@

fixes the ICE for me (though I really don't know what to put into dg-*
directives for the testcase, arm and aarch64 testcases are a puzzle for me).

[Bug target/99593] [11 Regression] arm Neon ICE when compiling firefox (skia) since r11-6708

[ktkachov at gcc dot gnu.org]

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

ktkachov at gcc dot gnu.org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
            Summary|[11 Regression] arm MVE ICE |[11 Regression] arm Neon
                   |when compiling firefox      |ICE when compiling firefox
                   |(skia) since r11-6708       |(skia) since r11-6708
           Keywords|                            |ice-on-valid-code

--- Comment #2 from ktkachov at gcc dot gnu.org ---
(Note it's a Neon ICE, not MVE). Yeah, that fix looks ok.
Chritophe, could you help here to write a testcase using arm_neon.h intrinsics
(rather than the builtins they decompose to)?

[Bug target/99593] [11 Regression] arm Neon ICE when compiling firefox (skia) since r11-6708

[jakub at gcc dot gnu.org]

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

--- Comment #3 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
I've tried to reproduce it with
typedef __simd128_int32_t int32x4_t;

void
foo (int32x4_t c)
{
  int32x4_t b = __builtin_neon_vdup_nv4si (3);
  register int32x4_t a __asm ("d16");
  asm volatile ("" : "=w" (a));
  a = a >> b;
  a |= c;
  asm volatile ("" : : "w" (a));
}
but it doesn't, for some reason the ICE needs a REG_EQUAL attribute on the
right shift into which we propagate the constant and LRA picks the constant
from there, but haven't managed to force that.

[Bug target/99593] [11 Regression] arm Neon ICE when compiling firefox (skia) since r11-6708

[clyon at gcc dot gnu.org]

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

Christophe Lyon <clyon at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Assignee|unassigned at gcc dot gnu.org      |clyon at gcc dot gnu.org

--- Comment #4 from Christophe Lyon <clyon at gcc dot gnu.org> ---
I have updated the test in the initial description to call intrinsics, added
relevant dg-* directives. Testing with several configurations under progress.


Is it really r11-6708 that introduced the problem? It doesn't modify vshq
unlike it's predecessor r11-6707-g7432f255b70811dafaf325d94036ac580891de69.

And that one only moves the offending pattern from neon.md to vec-common.md, so
the bug was probably already present?

As of -mtune=generic-armv7-a, I can reproduce the ICE with
-mcpu=generic-armv7-a, but I haven't found a -march equivalent
(-march=armv7-a+fp does not trigger the ICE)

[Bug target/99593] [11 Regression] arm Neon ICE when compiling firefox (skia) since r11-6708

[jakub at gcc dot gnu.org]

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

--- Comment #5 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
(In reply to Christophe Lyon from comment #4)
> I have updated the test in the initial description to call intrinsics, added
> relevant dg-* directives. Testing with several configurations under progress.
> 
> 
> Is it really r11-6708 that introduced the problem? It doesn't modify vshq
> unlike it's predecessor r11-6707-g7432f255b70811dafaf325d94036ac580891de69.

I think so, r11-6707 doesn't ICE.
In *.expand dump, the only r11-6707 to r11-6708 differences are 4 times
UNSPEC_ASHIFT_SIGNED -> VSHLQ_S
changes, and that is true up to ira dump too.
*.lra dump has:
-(insn 256 174 176 4 (set (reg:V4SI 52 d18 [331])
-        (const_vector:V4SI [
-                (const_int -3 [0xfffffffffffffffd]) repeated x4
-            ])) "ccLSOUAW.ii":66:45 1061 {*neon_movv4si}
-     (nil))
-(insn 176 256 177 4 (set (reg:V4SI 48 d16 [297])
+(insn 176 174 177 4 (set (reg:V4SI 48 d16 [297])
         (unspec:V4SI [
                 (reg:V4SI 48 d16 [296])
-                (reg:V4SI 52 d18 [331])
-            ] UNSPEC_ASHIFT_SIGNED)) "ccLSOUAW.ii":66:45 1350
{ashlv4si3_signed}
+                (const_vector:V4SI [
+                        (const_int -3 [0xfffffffffffffffd]) repeated x4
+                    ])
+            ] VSHLQ_S)) "ccLSOUAW.ii":66:45 510 {mve_vshlq_sv4si}
      (expr_list:REG_EQUAL (unspec:V4SI [
                 (reg:V4SI 48 d16 [296])
                 (const_vector:V4SI [
                         (const_int -3 [0xfffffffffffffffd]) repeated x4
                     ])
-            ] UNSPEC_ASHIFT_SIGNED)
+            ] VSHLQ_S)
         (nil)))
difference.

[Bug target/99593] [11 Regression] arm Neon ICE when compiling firefox (skia) since r11-6708

[jakub at gcc dot gnu.org]

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

--- Comment #6 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Before r11-6708 it was using different patterns - ashl<mode>3_signed - and
those
don't accept immediate CONST_VECTOR at all, while mve_vshlq_<supf><mode> does.

[Bug target/99593] [11 Regression] arm Neon ICE when compiling firefox (skia) since r11-6708

[clyon at gcc dot gnu.org]

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

--- Comment #7 from Christophe Lyon <clyon at gcc dot gnu.org> ---
Created attachment 50412
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=50412&action=edit
proposed testcase

Here is a proposal for a testcase derived from the initial description:
- added relevant dg-* directives
- replaced builtin calls with intrinsics

Jakub, Kyrill, is that OK with you?

[Bug target/99593] [11 Regression] arm Neon ICE when compiling firefox (skia) since r11-6708

[ktkachov at gcc dot gnu.org]

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

--- Comment #8 from ktkachov at gcc dot gnu.org ---
(In reply to Christophe Lyon from comment #7)
> Created attachment 50412 [details]
> proposed testcase
> 
> Here is a proposal for a testcase derived from the initial description:
> - added relevant dg-* directives
> - replaced builtin calls with intrinsics
> 
> Jakub, Kyrill, is that OK with you?

Thanks, that looks ok except:
+typedef __simd128_int32_t g;
+typedef __simd128_float32_t h;
+typedef __simd128_uint32_t i;

Can we replace them with the right ACLE types as well?

[Bug target/99593] [11 Regression] arm Neon ICE when compiling firefox (skia) since r11-6708

[jakub at gcc dot gnu.org]

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

--- Comment #9 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Comment on attachment 50412
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=50412
proposed testcase

Any reason not to replace
__simd128_int32_t with int32x4_t ,
__simd128_float32_t with float32x4_t and
__simd128_uint32_t with uint32x2_t ?
Drop the commented out __builtin_* names etc.?  Drop the (__builtin_neon_hi *)
cast?
Otherwise LGTM if it still FAILs without the above patch and PASSes with it,
but the final call is Kyrill's (or other ARM maintainers').

[Bug target/99593] [11 Regression] arm Neon ICE when compiling firefox (skia) since r11-6708

[ktkachov at gcc dot gnu.org]

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

--- Comment #10 from ktkachov at gcc dot gnu.org ---
(In reply to Jakub Jelinek from comment #9)
> Comment on attachment 50412 [details]
> proposed testcase
> 
> Any reason not to replace
> __simd128_int32_t with int32x4_t ,
> __simd128_float32_t with float32x4_t and
> __simd128_uint32_t with uint32x2_t ?
> Drop the commented out __builtin_* names etc.?  Drop the (__builtin_neon_hi
> *) cast?
> Otherwise LGTM if it still FAILs without the above patch and PASSes with it,
> but the final call is Kyrill's (or other ARM maintainers').

Indeed. Let's have a consolidated patch on gcc-patches for review.

[Bug target/99593] [11 Regression] arm Neon ICE when compiling firefox (skia) since r11-6708

[jakub at gcc dot gnu.org]

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

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

Ok, trying to test this overnight.

[Bug target/99593] [11 Regression] arm Neon ICE when compiling firefox (skia) since r11-6708

[clyon at gcc dot gnu.org]

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

--- Comment #12 from Christophe Lyon <clyon at gcc dot gnu.org> ---
I have tests in progress too (with and without the fix), except that I have
typedef uint32x4_t i;
instead of
typedef uint32x2_t i;

and I replaced the (__builtin_neon_hi *) cast with (int16_t*)

[Bug target/99593] [11 Regression] arm Neon ICE when compiling firefox (skia) since r11-6708

[jakub at gcc dot gnu.org]

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

--- Comment #13 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Oops, thanks for catching that.  Made those changes and restarted testing.

[Bug target/99593] [11 Regression] arm Neon ICE when compiling firefox (skia) since r11-6708

[clyon at gcc dot gnu.org]

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

--- Comment #14 from Christophe Lyon <clyon at gcc dot gnu.org> ---
I can confirm that the new test (as described in comment #12) works:
- alone it results in ICE in the relevant configurations (skipped otherwise)
- with the patch, it passes in the relevant configurations (skipped otherwise)

[Bug target/99593] [11 Regression] arm Neon ICE when compiling firefox (skia) since r11-6708

[cvs-commit at gcc dot gnu.org]

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

--- 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:009528d61c796608affd1eaa18ae31a3679eb46d

commit r11-7733-g009528d61c796608affd1eaa18ae31a3679eb46d
Author: Jakub Jelinek <jakub@redhat.com>
Date:   Fri Mar 19 13:48:44 2021 +0100

    arm: Fix mve_vshlq* [PR99593]

    As mentioned in the PR, before the
r11-6708-gbfab355012ca0f5219da8beb04f2fdaf757d34b7
    change v[al]shr<mode>3 expanders were expanding the shifts by register
    to gen_ashl<mode>3_{,un}signed which don't support immediate CONST_VECTOR
    shift amounts, but now expand to mve_vshlq_<supf><mode> which does.
    The testcase ICEs, because the constraint doesn't match the predicate and
    because LRA works solely with the constraints, so it can e.g. from
REG_EQUAL
    propagate there a CONST_VECTOR which matches the constraint but fails the
    predicate and only later on other passes will notice the predicate fails
    and ICE.

    Fixed by adding a constraint that matches the immediate part of the
    predicate.

            PR target/99593
            * config/arm/constraints.md (Ds): New constraint.
            * config/arm/vec-common.md (mve_vshlq_<supf><mode>): Use w,Ds
            constraint instead of w,Dm.

            * g++.target/arm/pr99593.C: New test.

[Bug target/99593] [11 Regression] arm Neon ICE when compiling firefox (skia) since r11-6708

[jakub at gcc dot gnu.org]

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

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

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

--- Comment #16 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Fixed.

[Bug target/99593] [11 Regression] arm Neon ICE when compiling firefox (skia) since r11-6708

[jakub at gcc dot gnu.org]

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

--- Comment #17 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Christophe, sorry for forgetting to add Co-authored-by, will fix it up in the
ChangeLog tomorrow by hand.

[Bug target/99593] [11 Regression] arm Neon ICE when compiling firefox (skia) since r11-6708

[clyon at gcc dot gnu.org]

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

--- Comment #18 from Christophe Lyon <clyon at gcc dot gnu.org> ---
Not a big deal if you forget, that's a detail :-)

[Bug target/99593] [11 Regression] arm Neon ICE when compiling firefox (skia) since r11-6708

[cvs-commit at gcc dot gnu.org]

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

--- Comment #19 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:6b1f841ce0ccf30eda7896ba5ab0aa94c72307b2

commit r11-7799-g6b1f841ce0ccf30eda7896ba5ab0aa94c72307b2
Author: Jakub Jelinek <jakub@redhat.com>
Date:   Tue Mar 23 16:29:47 2021 +0100

    Add forgotten attribution on PR target/99593 testcase.