Re: [PATCH] Avoid generate vblendps with ymm16+

[Hongtao Liu <crazylht@gmail.com>]

On Sat, Nov 11, 2023 at 4:11 AM Jakub Jelinek <jakub@redhat.com> wrote:
>
> On Thu, Nov 09, 2023 at 03:27:11PM +0800, Hongtao Liu wrote:
> > On Thu, Nov 9, 2023 at 3:15 PM Hu, Lin1 <lin1.hu@intel.com> wrote:
> > >
> > > This patch aims to avoid generate vblendps with ymm16+, And have
> > > bootstrapped and tested on x86_64-pc-linux-gnu{-m32,-m64}. Ok for trunk?
> > >
> > > gcc/ChangeLog:
> > >
> > >         PR target/112435
> > >         * config/i386/sse.md: Adding constraints to restrict the generation of
> > >         vblendps.
> > It should be "Don't output vblendps when evex sse reg or gpr32 is involved."
> > Others LGTM.
>
> I've missed this patch, so wrote my own today, and am wondering
>
> 1) if it isn't better to use separate alternative instead of
>    x86_evex_reg_mentioned_p, like in the patch below
vblendps doesn't support gpr32 which is checked by x86_evex_reg_mentioned_p.
we need to use xjm for operands[1], (I think we don't need to set
attribute addr to gpr16 for alternative 0 since the alternative 1 is
alway available and recog will match alternative1 when gpr32 is used)

> 2) why do you need the last two hunks in sse.md, both avx2_permv2ti and
>    *avx_vperm2f128<mode>_nozero insns only use x in constraints, never v,
>    so x86_evex_reg_mentioned_p ought to be always false there
true.
>
> Here is the untested patch, of course you have more testcases (though, I
> think it is better to test dg-do assemble with avx512vl target rather than
> dg-do compile and scan the assembler, after all, the problem was that it
> didn't assemble).
>
> 2023-11-10  Jakub Jelinek  <jakub@redhat.com>
>
>         PR target/112435
>         * config/i386/sse.md (avx512vl_shuf_<shuffletype>32x4_1<mask_name>,
>         <mask_codefor>avx512dq_shuf_<shuffletype>64x2_1<mask_name>): Add
>         alternative with just x instead of v constraints and use vblendps
>         as optimization only with that alternative.
>
>         * gcc.target/i386/avx512vl-pr112435.c: New test.
>
> --- gcc/config/i386/sse.md.jj   2023-11-09 09:04:18.616543403 +0100
> +++ gcc/config/i386/sse.md      2023-11-10 15:56:44.138499931 +0100
> @@ -19235,11 +19235,11 @@ (define_expand "avx512dq_shuf_<shufflety
>  })
>
>  (define_insn "<mask_codefor>avx512dq_shuf_<shuffletype>64x2_1<mask_name>"
> -  [(set (match_operand:VI8F_256 0 "register_operand" "=v")
> +  [(set (match_operand:VI8F_256 0 "register_operand" "=x,v")
>         (vec_select:VI8F_256
>           (vec_concat:<ssedoublemode>
> -           (match_operand:VI8F_256 1 "register_operand" "v")
> -           (match_operand:VI8F_256 2 "nonimmediate_operand" "vm"))
> +           (match_operand:VI8F_256 1 "register_operand" "x,v")
> +           (match_operand:VI8F_256 2 "nonimmediate_operand" "xm,vm"))
>           (parallel [(match_operand 3 "const_0_to_3_operand")
>                      (match_operand 4 "const_0_to_3_operand")
>                      (match_operand 5 "const_4_to_7_operand")
> @@ -19254,7 +19254,7 @@ (define_insn "<mask_codefor>avx512dq_shu
>    mask = INTVAL (operands[3]) / 2;
>    mask |= (INTVAL (operands[5]) - 4) / 2 << 1;
>    operands[3] = GEN_INT (mask);
> -  if (INTVAL (operands[3]) == 2 && !<mask_applied>)
> +  if (INTVAL (operands[3]) == 2 && !<mask_applied> && which_alternative == 0)
>      return "vblendps\t{$240, %2, %1, %0|%0, %1, %2, 240}";
>    return "vshuf<shuffletype>64x2\t{%3, %2, %1, %0<mask_operand7>|%0<mask_operand7>, %1, %2, %3}";
>  }
> @@ -19386,11 +19386,11 @@ (define_expand "avx512vl_shuf_<shufflety
>  })
>
>  (define_insn "avx512vl_shuf_<shuffletype>32x4_1<mask_name>"
> -  [(set (match_operand:VI4F_256 0 "register_operand" "=v")
> +  [(set (match_operand:VI4F_256 0 "register_operand" "=x,v")
>         (vec_select:VI4F_256
>           (vec_concat:<ssedoublemode>
> -           (match_operand:VI4F_256 1 "register_operand" "v")
> -           (match_operand:VI4F_256 2 "nonimmediate_operand" "vm"))
> +           (match_operand:VI4F_256 1 "register_operand" "x,v")
> +           (match_operand:VI4F_256 2 "nonimmediate_operand" "xm,vm"))
>           (parallel [(match_operand 3 "const_0_to_7_operand")
>                      (match_operand 4 "const_0_to_7_operand")
>                      (match_operand 5 "const_0_to_7_operand")
> @@ -19414,7 +19414,7 @@ (define_insn "avx512vl_shuf_<shuffletype
>    mask |= (INTVAL (operands[7]) - 8) / 4 << 1;
>    operands[3] = GEN_INT (mask);
>
> -  if (INTVAL (operands[3]) == 2 && !<mask_applied>)
> +  if (INTVAL (operands[3]) == 2 && !<mask_applied> && which_alternative == 0)
>      return "vblendps\t{$240, %2, %1, %0|%0, %1, %2, 240}";
>
>    return "vshuf<shuffletype>32x4\t{%3, %2, %1, %0<mask_operand11>|%0<mask_operand11>, %1, %2, %3}";
> --- gcc/testsuite/gcc.target/i386/avx512vl-pr112435.c.jj        2023-11-10 16:04:21.708046771 +0100
> +++ gcc/testsuite/gcc.target/i386/avx512vl-pr112435.c   2023-11-10 16:03:51.053479094 +0100
> @@ -0,0 +1,13 @@
> +/* PR target/112435 */
> +/* { dg-do assemble { target { avx512vl && { ! ia32 } } } } */
> +/* { dg-options "-mavx512vl -O2" } */
> +
> +#include <x86intrin.h>
> +
> +__m256i
> +foo (__m256i a, __m256i b)
> +{
> +  register __m256i c __asm__("ymm16") = a;
> +  asm ("" : "+v" (c));
> +  return _mm256_shuffle_i32x4 (c, b, 2);
> +}
>
>         Jakub
>


-- 
BR,
Hongtao