Re: [PATCH] Optimize vlddqu + inserti128 to vbroadcasti128

[Uros Bizjak <ubizjak@gmail.com>]

On Wed, Aug 2, 2023 at 3:33 AM liuhongt <hongtao.liu@intel.com> wrote:
>
> In [1], I propose a patch to generate vmovdqu for all vlddqu intrinsics
> after AVX2, it's rejected as
> > The instruction is reachable only as __builtin_ia32_lddqu* (aka
> > _mm_lddqu_si*), so it was chosen by the programmer for a reason. I
> > think that in this case, the compiler should not be too smart and
> > change the instruction behind the programmer's back. The caveats are
> > also explained at length in the ISA manual.
>
> So the patch is more conservative, only optimize vlddqu + vinserti128
> to vbroadcasti128.
> vlddqu + vinserti128 will use shuffle port in addition to load port
> comparing to vbroadcasti128, For latency perspective,vbroadcasti is no
> worse than vlddqu + vinserti128.
>
> [1] https://gcc.gnu.org/pipermail/gcc-patches/2023-July/625122.html
>
> Bootstrapped and regtested on x86_64-linux-gnu{-m32,}.
> Ok for trunk?
>
> gcc/ChangeLog:
>
>         * config/i386/sse.md (*avx2_lddqu_inserti_to_bcasti): New
>         pre_reload define_insn_and_split.
>
> gcc/testsuite/ChangeLog:
>
>         * gcc.target/i386/vlddqu_vinserti128.c: New test.

OK with a small change bellow.

Thanks,
Uros.

> ---
>  gcc/config/i386/sse.md                         | 18 ++++++++++++++++++
>  .../gcc.target/i386/vlddqu_vinserti128.c       | 11 +++++++++++
>  2 files changed, 29 insertions(+)
>  create mode 100644 gcc/testsuite/gcc.target/i386/vlddqu_vinserti128.c
>
> diff --git a/gcc/config/i386/sse.md b/gcc/config/i386/sse.md
> index 2d81347c7b6..4bdd2b43ba7 100644
> --- a/gcc/config/i386/sse.md
> +++ b/gcc/config/i386/sse.md
> @@ -26600,6 +26600,24 @@ (define_insn "avx2_vbroadcasti128_<mode>"
>     (set_attr "prefix" "vex,evex,evex")
>     (set_attr "mode" "OI")])
>
> +;; optimize vlddqu + vinserti128 to vbroadcasti128, the former will use
> +;; extra shuffle port in addition to load port than the latter.
> +;; For latency perspective,vbroadcasti is no worse.
> +(define_insn_and_split "avx2_lddqu_inserti_to_bcasti"
> +  [(set (match_operand:V4DI 0 "register_operand" "=x,v,v")
> +       (vec_concat:V4DI
> +         (subreg:V2DI
> +           (unspec:V16QI [(match_operand:V16QI 1 "memory_operand")]
> +                         UNSPEC_LDDQU) 0)
> +         (subreg:V2DI (unspec:V16QI [(match_dup 1)]
> +                         UNSPEC_LDDQU) 0)))]
> +  "TARGET_AVX2 && ix86_pre_reload_split ()"
> +  "#"
> +  "&& 1"
> +  [(set (match_dup 0)
> +       (vec_concat:V4DI (match_dup 1) (match_dup 1)))]
> +  "operands[1] = adjust_address (operands[1], V2DImode, 0);")

No need to validate address before reload, adjust_address_nv can be used.

> +
>  ;; Modes handled by AVX vec_dup patterns.
>  (define_mode_iterator AVX_VEC_DUP_MODE
>    [V8SI V8SF V4DI V4DF])
> diff --git a/gcc/testsuite/gcc.target/i386/vlddqu_vinserti128.c b/gcc/testsuite/gcc.target/i386/vlddqu_vinserti128.c
> new file mode 100644
> index 00000000000..29699a5fa7f
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/i386/vlddqu_vinserti128.c
> @@ -0,0 +1,11 @@
> +/* { dg-do compile } */
> +/* { dg-options "-mavx2 -O2" } */
> +/* { dg-final { scan-assembler-times "vbroadcasti128" 1 } } */
> +/* { dg-final { scan-assembler-not {(?n)vlddqu.*xmm} } } */
> +
> +#include <immintrin.h>
> +__m256i foo(void *data) {
> +    __m128i X1 = _mm_lddqu_si128((__m128i*)data);
> +    __m256i V1 = _mm256_broadcastsi128_si256 (X1);
> +    return V1;
> +}
> --
> 2.39.1.388.g2fc9e9ca3c
>