diff mbox series

RISC-V: THEAD: Fix ICE caused by split optimizations for XTheadFMemIdx.

Message ID 20240111112304.1398-1-jinma@linux.alibaba.com
State New
Headers show
Series RISC-V: THEAD: Fix ICE caused by split optimizations for XTheadFMemIdx. | expand

Commit Message

Jin Ma Jan. 11, 2024, 11:23 a.m. UTC
Due to the premature split optimizations for XTheadFMemIdx, GPR
is allocated when reload allocates registers, resulting in the
following insn.

(insn 66 21 64 5 (set (reg:DF 14 a4 [orig:136 <retval> ] [136])
        (mem:DF (plus:SI (reg/f:SI 15 a5 [141])
                (ashift:SI (reg/v:SI 10 a0 [orig:137 i ] [137])
                    (const_int 3 [0x3]))) [0  S8 A64])) 218 {*movdf_hardfloat_rv32}
     (nil))

Since we currently do not support adjustments to th_m_mir/th_m_miu,
which will trigger ICE. So it is recommended to place the split
optimizations after reload to ensure FPR when registers are allocated.

gcc/ChangeLog:

	* config/riscv/thead.md: Add limits for splits.

gcc/testsuite/ChangeLog:

	* gcc.target/riscv/xtheadfmemidx-medany.c: New test.
---
 gcc/config/riscv/thead.md                     | 22 ++++++++---
 .../gcc.target/riscv/xtheadfmemidx-medany.c   | 38 +++++++++++++++++++
 2 files changed, 54 insertions(+), 6 deletions(-)
 create mode 100644 gcc/testsuite/gcc.target/riscv/xtheadfmemidx-medany.c

Comments

Kito Cheng Jan. 11, 2024, 3:35 p.m. UTC | #1
LGTM

On Thu, Jan 11, 2024 at 7:23 PM Jin Ma <jinma@linux.alibaba.com> wrote:
>
> Due to the premature split optimizations for XTheadFMemIdx, GPR
> is allocated when reload allocates registers, resulting in the
> following insn.
>
> (insn 66 21 64 5 (set (reg:DF 14 a4 [orig:136 <retval> ] [136])
>         (mem:DF (plus:SI (reg/f:SI 15 a5 [141])
>                 (ashift:SI (reg/v:SI 10 a0 [orig:137 i ] [137])
>                     (const_int 3 [0x3]))) [0  S8 A64])) 218 {*movdf_hardfloat_rv32}
>      (nil))
>
> Since we currently do not support adjustments to th_m_mir/th_m_miu,
> which will trigger ICE. So it is recommended to place the split
> optimizations after reload to ensure FPR when registers are allocated.
>
> gcc/ChangeLog:
>
>         * config/riscv/thead.md: Add limits for splits.
>
> gcc/testsuite/ChangeLog:
>
>         * gcc.target/riscv/xtheadfmemidx-medany.c: New test.
> ---
>  gcc/config/riscv/thead.md                     | 22 ++++++++---
>  .../gcc.target/riscv/xtheadfmemidx-medany.c   | 38 +++++++++++++++++++
>  2 files changed, 54 insertions(+), 6 deletions(-)
>  create mode 100644 gcc/testsuite/gcc.target/riscv/xtheadfmemidx-medany.c
>
> diff --git a/gcc/config/riscv/thead.md b/gcc/config/riscv/thead.md
> index e370774d518..5c7d4beb1b6 100644
> --- a/gcc/config/riscv/thead.md
> +++ b/gcc/config/riscv/thead.md
> @@ -933,14 +933,17 @@ (define_insn_and_split "*th_fmemidx_I_a"
>     && pow2p_hwi (INTVAL (operands[2]))
>     && IN_RANGE (exact_log2 (INTVAL (operands[2])), 1, 3)"
>    "#"
> -  "&& 1"
> +  "&& reload_completed"
>    [(set (match_dup 0)
>          (mem:TH_M_NOEXTF (plus:X
>            (match_dup 3)
>            (ashift:X (match_dup 1) (match_dup 2)))))]
>    { operands[2] = GEN_INT (exact_log2 (INTVAL (operands [2])));
>    }
> -)
> +  [(set_attr "move_type" "fpload")
> +   (set_attr "mode" "<UNITMODE>")
> +   (set_attr "type" "fmove")
> +   (set (attr "length") (const_int 16))])
>
>  (define_insn_and_split "*th_fmemidx_I_c"
>    [(set (mem:TH_M_ANYF (plus:X
> @@ -977,7 +980,7 @@ (define_insn_and_split "*th_fmemidx_US_a"
>     && CONST_INT_P (operands[3])
>     && (INTVAL (operands[3]) >> exact_log2 (INTVAL (operands[2]))) == 0xffffffff"
>    "#"
> -  "&& 1"
> +  "&& reload_completed"
>    [(set (match_dup 0)
>          (mem:TH_M_NOEXTF (plus:DI
>            (match_dup 4)
> @@ -985,7 +988,10 @@ (define_insn_and_split "*th_fmemidx_US_a"
>    { operands[1] = gen_lowpart (SImode, operands[1]);
>      operands[2] = GEN_INT (exact_log2 (INTVAL (operands [2])));
>    }
> -)
> +  [(set_attr "move_type" "fpload")
> +   (set_attr "mode" "<UNITMODE>")
> +   (set_attr "type" "fmove")
> +   (set (attr "length") (const_int 16))])
>
>  (define_insn_and_split "*th_fmemidx_US_c"
>    [(set (mem:TH_M_ANYF (plus:DI
> @@ -1020,12 +1026,16 @@ (define_insn_and_split "*th_fmemidx_UZ_a"
>    "TARGET_64BIT && TARGET_XTHEADMEMIDX && TARGET_XTHEADFMEMIDX
>     && (!HARD_REGISTER_NUM_P (REGNO (operands[0])) || HARDFP_REG_P (REGNO (operands[0])))"
>    "#"
> -  "&& 1"
> +  "&& reload_completed"
>    [(set (match_dup 0)
>          (mem:TH_M_NOEXTF (plus:DI
>            (match_dup 2)
>            (zero_extend:DI (match_dup 1)))))]
> -)
> +  ""
> +  [(set_attr "move_type" "fpload")
> +   (set_attr "mode" "<UNITMODE>")
> +   (set_attr "type" "fmove")
> +   (set (attr "length") (const_int 16))])
>
>  (define_insn_and_split "*th_fmemidx_UZ_c"
>    [(set (mem:TH_M_ANYF (plus:DI
> diff --git a/gcc/testsuite/gcc.target/riscv/xtheadfmemidx-medany.c b/gcc/testsuite/gcc.target/riscv/xtheadfmemidx-medany.c
> new file mode 100644
> index 00000000000..0c8060d0632
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/riscv/xtheadfmemidx-medany.c
> @@ -0,0 +1,38 @@
> +/* { dg-do compile } */
> +/* { dg-skip-if "" { *-*-* } { "-O0" "-O1" "-O3" "-Og" "-Os" "-Oz"} } */
> +/* { dg-options "-march=rv32gc_xtheadfmemidx_xtheadfmv_xtheadmemidx -mabi=ilp32d -mcmodel=medany -O2" } */
> +
> +typedef union {
> +  double v;
> +  unsigned w;
> +} my_t;
> +
> +double z;
> +
> +double foo (int i, int j)
> +{
> +
> +  if (j)
> +    {
> +      switch (i)
> +       {
> +       case 0:
> +         return 1;
> +       case 1:
> +         return 0;
> +       case 2:
> +         return 3.0;
> +       }
> +    }
> +
> +  if (i == 1)
> +    {
> +      my_t u;
> +      u.v = z;
> +      u.w = 1;
> +      z = u.v;
> +    }
> +  return z;
> +}
> +
> +/* { dg-final { scan-assembler-times {\mth\.flrd\M} 1 } } */
> --
> 2.17.1
>
Christoph Müllner Jan. 11, 2024, 6:45 p.m. UTC | #2
On Thu, Jan 11, 2024 at 4:36 PM Kito Cheng <kito.cheng@gmail.com> wrote:
>
> LGTM
>
> On Thu, Jan 11, 2024 at 7:23 PM Jin Ma <jinma@linux.alibaba.com> wrote:
> >
> > Due to the premature split optimizations for XTheadFMemIdx, GPR
> > is allocated when reload allocates registers, resulting in the
> > following insn.

LGTM.
This was most likely not detected so far, because it only affects rv32
(test focus on xthead(f)memidx was rv64).
I've rebased, retested (no regressions) and pushed.

Thanks,
Christoph

> >
> > (insn 66 21 64 5 (set (reg:DF 14 a4 [orig:136 <retval> ] [136])
> >         (mem:DF (plus:SI (reg/f:SI 15 a5 [141])
> >                 (ashift:SI (reg/v:SI 10 a0 [orig:137 i ] [137])
> >                     (const_int 3 [0x3]))) [0  S8 A64])) 218 {*movdf_hardfloat_rv32}
> >      (nil))
> >
> > Since we currently do not support adjustments to th_m_mir/th_m_miu,
> > which will trigger ICE. So it is recommended to place the split
> > optimizations after reload to ensure FPR when registers are allocated.
> >
> > gcc/ChangeLog:
> >
> >         * config/riscv/thead.md: Add limits for splits.
> >
> > gcc/testsuite/ChangeLog:
> >
> >         * gcc.target/riscv/xtheadfmemidx-medany.c: New test.
> > ---
> >  gcc/config/riscv/thead.md                     | 22 ++++++++---
> >  .../gcc.target/riscv/xtheadfmemidx-medany.c   | 38 +++++++++++++++++++
> >  2 files changed, 54 insertions(+), 6 deletions(-)
> >  create mode 100644 gcc/testsuite/gcc.target/riscv/xtheadfmemidx-medany.c
> >
> > diff --git a/gcc/config/riscv/thead.md b/gcc/config/riscv/thead.md
> > index e370774d518..5c7d4beb1b6 100644
> > --- a/gcc/config/riscv/thead.md
> > +++ b/gcc/config/riscv/thead.md
> > @@ -933,14 +933,17 @@ (define_insn_and_split "*th_fmemidx_I_a"
> >     && pow2p_hwi (INTVAL (operands[2]))
> >     && IN_RANGE (exact_log2 (INTVAL (operands[2])), 1, 3)"
> >    "#"
> > -  "&& 1"
> > +  "&& reload_completed"
> >    [(set (match_dup 0)
> >          (mem:TH_M_NOEXTF (plus:X
> >            (match_dup 3)
> >            (ashift:X (match_dup 1) (match_dup 2)))))]
> >    { operands[2] = GEN_INT (exact_log2 (INTVAL (operands [2])));
> >    }
> > -)
> > +  [(set_attr "move_type" "fpload")
> > +   (set_attr "mode" "<UNITMODE>")
> > +   (set_attr "type" "fmove")
> > +   (set (attr "length") (const_int 16))])
> >
> >  (define_insn_and_split "*th_fmemidx_I_c"
> >    [(set (mem:TH_M_ANYF (plus:X
> > @@ -977,7 +980,7 @@ (define_insn_and_split "*th_fmemidx_US_a"
> >     && CONST_INT_P (operands[3])
> >     && (INTVAL (operands[3]) >> exact_log2 (INTVAL (operands[2]))) == 0xffffffff"
> >    "#"
> > -  "&& 1"
> > +  "&& reload_completed"
> >    [(set (match_dup 0)
> >          (mem:TH_M_NOEXTF (plus:DI
> >            (match_dup 4)
> > @@ -985,7 +988,10 @@ (define_insn_and_split "*th_fmemidx_US_a"
> >    { operands[1] = gen_lowpart (SImode, operands[1]);
> >      operands[2] = GEN_INT (exact_log2 (INTVAL (operands [2])));
> >    }
> > -)
> > +  [(set_attr "move_type" "fpload")
> > +   (set_attr "mode" "<UNITMODE>")
> > +   (set_attr "type" "fmove")
> > +   (set (attr "length") (const_int 16))])
> >
> >  (define_insn_and_split "*th_fmemidx_US_c"
> >    [(set (mem:TH_M_ANYF (plus:DI
> > @@ -1020,12 +1026,16 @@ (define_insn_and_split "*th_fmemidx_UZ_a"
> >    "TARGET_64BIT && TARGET_XTHEADMEMIDX && TARGET_XTHEADFMEMIDX
> >     && (!HARD_REGISTER_NUM_P (REGNO (operands[0])) || HARDFP_REG_P (REGNO (operands[0])))"
> >    "#"
> > -  "&& 1"
> > +  "&& reload_completed"
> >    [(set (match_dup 0)
> >          (mem:TH_M_NOEXTF (plus:DI
> >            (match_dup 2)
> >            (zero_extend:DI (match_dup 1)))))]
> > -)
> > +  ""
> > +  [(set_attr "move_type" "fpload")
> > +   (set_attr "mode" "<UNITMODE>")
> > +   (set_attr "type" "fmove")
> > +   (set (attr "length") (const_int 16))])
> >
> >  (define_insn_and_split "*th_fmemidx_UZ_c"
> >    [(set (mem:TH_M_ANYF (plus:DI
> > diff --git a/gcc/testsuite/gcc.target/riscv/xtheadfmemidx-medany.c b/gcc/testsuite/gcc.target/riscv/xtheadfmemidx-medany.c
> > new file mode 100644
> > index 00000000000..0c8060d0632
> > --- /dev/null
> > +++ b/gcc/testsuite/gcc.target/riscv/xtheadfmemidx-medany.c
> > @@ -0,0 +1,38 @@
> > +/* { dg-do compile } */
> > +/* { dg-skip-if "" { *-*-* } { "-O0" "-O1" "-O3" "-Og" "-Os" "-Oz"} } */
> > +/* { dg-options "-march=rv32gc_xtheadfmemidx_xtheadfmv_xtheadmemidx -mabi=ilp32d -mcmodel=medany -O2" } */
> > +
> > +typedef union {
> > +  double v;
> > +  unsigned w;
> > +} my_t;
> > +
> > +double z;
> > +
> > +double foo (int i, int j)
> > +{
> > +
> > +  if (j)
> > +    {
> > +      switch (i)
> > +       {
> > +       case 0:
> > +         return 1;
> > +       case 1:
> > +         return 0;
> > +       case 2:
> > +         return 3.0;
> > +       }
> > +    }
> > +
> > +  if (i == 1)
> > +    {
> > +      my_t u;
> > +      u.v = z;
> > +      u.w = 1;
> > +      z = u.v;
> > +    }
> > +  return z;
> > +}
> > +
> > +/* { dg-final { scan-assembler-times {\mth\.flrd\M} 1 } } */
> > --
> > 2.17.1
> >
diff mbox series

Patch

diff --git a/gcc/config/riscv/thead.md b/gcc/config/riscv/thead.md
index e370774d518..5c7d4beb1b6 100644
--- a/gcc/config/riscv/thead.md
+++ b/gcc/config/riscv/thead.md
@@ -933,14 +933,17 @@  (define_insn_and_split "*th_fmemidx_I_a"
    && pow2p_hwi (INTVAL (operands[2]))
    && IN_RANGE (exact_log2 (INTVAL (operands[2])), 1, 3)"
   "#"
-  "&& 1"
+  "&& reload_completed"
   [(set (match_dup 0)
         (mem:TH_M_NOEXTF (plus:X
           (match_dup 3)
           (ashift:X (match_dup 1) (match_dup 2)))))]
   { operands[2] = GEN_INT (exact_log2 (INTVAL (operands [2])));
   }
-)
+  [(set_attr "move_type" "fpload")
+   (set_attr "mode" "<UNITMODE>")
+   (set_attr "type" "fmove")
+   (set (attr "length") (const_int 16))])
 
 (define_insn_and_split "*th_fmemidx_I_c"
   [(set (mem:TH_M_ANYF (plus:X
@@ -977,7 +980,7 @@  (define_insn_and_split "*th_fmemidx_US_a"
    && CONST_INT_P (operands[3])
    && (INTVAL (operands[3]) >> exact_log2 (INTVAL (operands[2]))) == 0xffffffff"
   "#"
-  "&& 1"
+  "&& reload_completed"
   [(set (match_dup 0)
         (mem:TH_M_NOEXTF (plus:DI
           (match_dup 4)
@@ -985,7 +988,10 @@  (define_insn_and_split "*th_fmemidx_US_a"
   { operands[1] = gen_lowpart (SImode, operands[1]);
     operands[2] = GEN_INT (exact_log2 (INTVAL (operands [2])));
   }
-)
+  [(set_attr "move_type" "fpload")
+   (set_attr "mode" "<UNITMODE>")
+   (set_attr "type" "fmove")
+   (set (attr "length") (const_int 16))])
 
 (define_insn_and_split "*th_fmemidx_US_c"
   [(set (mem:TH_M_ANYF (plus:DI
@@ -1020,12 +1026,16 @@  (define_insn_and_split "*th_fmemidx_UZ_a"
   "TARGET_64BIT && TARGET_XTHEADMEMIDX && TARGET_XTHEADFMEMIDX
    && (!HARD_REGISTER_NUM_P (REGNO (operands[0])) || HARDFP_REG_P (REGNO (operands[0])))"
   "#"
-  "&& 1"
+  "&& reload_completed"
   [(set (match_dup 0)
         (mem:TH_M_NOEXTF (plus:DI
           (match_dup 2)
           (zero_extend:DI (match_dup 1)))))]
-)
+  ""
+  [(set_attr "move_type" "fpload")
+   (set_attr "mode" "<UNITMODE>")
+   (set_attr "type" "fmove")
+   (set (attr "length") (const_int 16))])
 
 (define_insn_and_split "*th_fmemidx_UZ_c"
   [(set (mem:TH_M_ANYF (plus:DI
diff --git a/gcc/testsuite/gcc.target/riscv/xtheadfmemidx-medany.c b/gcc/testsuite/gcc.target/riscv/xtheadfmemidx-medany.c
new file mode 100644
index 00000000000..0c8060d0632
--- /dev/null
+++ b/gcc/testsuite/gcc.target/riscv/xtheadfmemidx-medany.c
@@ -0,0 +1,38 @@ 
+/* { dg-do compile } */
+/* { dg-skip-if "" { *-*-* } { "-O0" "-O1" "-O3" "-Og" "-Os" "-Oz"} } */
+/* { dg-options "-march=rv32gc_xtheadfmemidx_xtheadfmv_xtheadmemidx -mabi=ilp32d -mcmodel=medany -O2" } */
+
+typedef union {
+  double v;
+  unsigned w;
+} my_t;
+
+double z;
+
+double foo (int i, int j)
+{
+
+  if (j)
+    {
+      switch (i)
+	{
+	case 0:
+	  return 1;
+	case 1:
+	  return 0;
+	case 2:
+	  return 3.0;
+	}
+    }
+
+  if (i == 1)
+    {
+      my_t u;
+      u.v = z;
+      u.w = 1;
+      z = u.v;
+    }
+  return z;
+}
+
+/* { dg-final { scan-assembler-times {\mth\.flrd\M} 1 } } */