Re: [PATCH] [i386] APX: Fix ICE due to movti postreload splitter [PR112394]

[Hongtao Liu <crazylht@gmail.com>]

On Tue, Nov 7, 2023 at 3:33 PM Hongyu Wang <hongyu.wang@intel.com> wrote:
>
> Hi,
>
> When APX EGPR enabled, the TImode move pattern *movti_internal allows
> move between gpr and sse reg using constraint pair ("r","Yd"). Then a
> post-reload splitter transform such move to vec_extractv2di, while under
> -msse4.1 -mno-avx EGPR is not allowed for its enabled alternative, which
> caused ICE that insn does not match the constraint. To prevent such ICE,
> we need to adjust the constraint correspond to "Yd". Add a new "jc"
> constraint to disable EGPR under -mno-avx.
>
> Bootstrapped and regtseted on x86_64-pc-linux-gnu{-m32,}.
>
> OK for trunk?
LGTM.
>
> gcc/ChangeLog:
>
>         PR target/112394
>         * config/i386/constraints.md (jc): New constraint that prohibits
>         EGPR on -mno-avx.
>         * config/i386/i386.md (*movdi_internal): Change r constraint
>         corresponds to Yd.
>         (*movti_internal): Likewise.
>
> gcc/testsuite/ChangeLog:
>
>         PR target/112394
>         * gcc.target/i386/pr112394.c: New test.
> ---
>  gcc/config/i386/constraints.md           |  3 +++
>  gcc/config/i386/i386.md                  |  8 ++++----
>  gcc/testsuite/gcc.target/i386/pr112394.c | 24 ++++++++++++++++++++++++
>  3 files changed, 31 insertions(+), 4 deletions(-)
>  create mode 100644 gcc/testsuite/gcc.target/i386/pr112394.c
>
> diff --git a/gcc/config/i386/constraints.md b/gcc/config/i386/constraints.md
> index f6275740eb2..74c2f0f2d32 100644
> --- a/gcc/config/i386/constraints.md
> +++ b/gcc/config/i386/constraints.md
> @@ -434,3 +434,6 @@ (define_address_constraint "jb"
>    (and (match_operand 0 "vsib_address_operand")
>         (not (and (match_test "TARGET_APX_EGPR")
>                  (match_test "x86_extended_rex2reg_mentioned_p (op)")))))
> +
> +(define_register_constraint  "jc"
> + "TARGET_APX_EGPR && !TARGET_AVX ? GENERAL_GPR16 : GENERAL_REGS")
> diff --git a/gcc/config/i386/i386.md b/gcc/config/i386/i386.md
> index ecc74e9994e..ec39c2dd512 100644
> --- a/gcc/config/i386/i386.md
> +++ b/gcc/config/i386/i386.md
> @@ -2454,8 +2454,8 @@ (define_insn "*movoi_internal_avx"
>     (set_attr "mode" "OI")])
>
>  (define_insn "*movti_internal"
> -  [(set (match_operand:TI 0 "nonimmediate_operand" "=!r ,o ,v,v ,v ,m,?r,?Yd")
> -       (match_operand:TI 1 "general_operand"      "riFo,re,C,BC,vm,v,Yd,r"))]
> +  [(set (match_operand:TI 0 "nonimmediate_operand" "=!r ,o ,v,v ,v ,m,?jc,?Yd")
> +       (match_operand:TI 1 "general_operand"      "riFo,re,C,BC,vm,v,Yd,jc"))]
>    "(TARGET_64BIT
>      && !(MEM_P (operands[0]) && MEM_P (operands[1])))
>     || (TARGET_SSE
> @@ -2537,9 +2537,9 @@ (define_split
>
>  (define_insn "*movdi_internal"
>    [(set (match_operand:DI 0 "nonimmediate_operand"
> -    "=r  ,o  ,r,r  ,r,m ,*y,*y,?*y,?m,?r,?*y,?Yv,?v,?v,m ,m,?r ,?*Yd,?r,?v,?*y,?*x,*k,*k  ,*r,*m,*k")
> +    "=r  ,o  ,r,r  ,r,m ,*y,*y,?*y,?m,?r,?*y,?Yv,?v,?v,m ,m,?jc,?*Yd,?r,?v,?*y,?*x,*k,*k  ,*r,*m,*k")
>         (match_operand:DI 1 "general_operand"
> -    "riFo,riF,Z,rem,i,re,C ,*y,Bk ,*y,*y,r  ,C  ,?v,Bk,?v,v,*Yd,r   ,?v,r  ,*x ,*y ,*r,*kBk,*k,*k,CBC"))]
> +    "riFo,riF,Z,rem,i,re,C ,*y,Bk ,*y,*y,r  ,C  ,?v,Bk,?v,v,*Yd,jc  ,?v,r  ,*x ,*y ,*r,*kBk,*k,*k,CBC"))]
>    "!(MEM_P (operands[0]) && MEM_P (operands[1]))
>     && ix86_hardreg_mov_ok (operands[0], operands[1])"
>  {
> diff --git a/gcc/testsuite/gcc.target/i386/pr112394.c b/gcc/testsuite/gcc.target/i386/pr112394.c
> new file mode 100644
> index 00000000000..c582f6ea6bd
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/i386/pr112394.c
> @@ -0,0 +1,24 @@
> +/* PR target/112394 */
> +/* { dg-do compile { target { ! ia32 } } } */
> +/* { dg-options "-msse4.1 -m64 -O -mapxf" } */
> +
> +typedef int __attribute__((__vector_size__ (8))) A;
> +typedef int __attribute__((__vector_size__ (16))) B;
> +typedef char __attribute__((__vector_size__ (4))) C;
> +typedef char __attribute__((__vector_size__ (32))) D;
> +typedef _Complex __int128 CU;
> +typedef _Float16 __attribute__((__vector_size__ (8))) F;
> +D d;
> +B b;
> +CU gcu;
> +
> +int
> +foo (char c, int, int, int, int, CU cu, int x)
> +{
> +  d /= c | d;
> +  F f = __builtin_convertvector (b, F);
> +  cu /= gcu;
> +  A a = (A) f;
> +  int i = cu + x;
> +  return ((C) a[0])[1] + i + c;
> +}
> --
> 2.31.1
>


-- 
BR,
Hongtao