Re: [PATCH i386 11/8] [AVX512] Add missing packed PF gathers/scatters, rename load/store.

[Uros Bizjak <ubizjak at gmail dot com>]

On Tue, Jan 14, 2014 at 7:13 AM, Kirill Yukhin <kirill.yukhin@gmail.com> wrote:
> Hello,
> This patch introduces missing AVX-512PF intrinsics and tests.
> It also renames store/load intrinsics according to EAS.
>
> gcc/
>         * config/i386/avx512fintrin.h (_mm512_loadu_si512): Rename.
>         (_mm512_storeu_si512): Ditto.
>         * config/i386/avx512pfintrin.h (_mm512_mask_prefetch_i32gather_pd): New.
>         (_mm512_mask_prefetch_i64gather_pd): Ditto.
>         (_mm512_prefetch_i32scatter_pd): Ditto.
>         (_mm512_mask_prefetch_i32scatter_pd): Ditto.
>         (_mm512_prefetch_i64scatter_pd): Ditto.
>         (_mm512_mask_prefetch_i64scatter_pd): Ditto.
>         (_mm512_mask_prefetch_i32gather_ps): Fix operand type.
>         (_mm512_mask_prefetch_i64gather_ps): Ditto.
>         (_mm512_prefetch_i32scatter_ps): Ditto.
>         (_mm512_mask_prefetch_i32scatter_ps): Ditto.
>         (_mm512_prefetch_i64scatter_ps): Ditto.
>         (_mm512_mask_prefetch_i64scatter_ps): Ditto.
>         * config/i386/i386-builtin-types.def: Define
>         VOID_FTYPE_QI_V8SI_PCINT64_INT_INT and VOID_FTYPE_QI_V8DI_PCINT64_INT_INT.
>         * config/i386/i386.c (ix86_builtins): Define IX86_BUILTIN_GATHERPFQPD,
>         IX86_BUILTIN_GATHERPFDPD, IX86_BUILTIN_SCATTERPFDPD,
>         IX86_BUILTIN_SCATTERPFQPD.
>         (ix86_init_mmx_sse_builtins): Define __builtin_ia32_gatherpfdpd,
>         __builtin_ia32_gatherpfdps, __builtin_ia32_gatherpfqpd,
>         __builtin_ia32_gatherpfqps, __builtin_ia32_scatterpfdpd,
>         __builtin_ia32_scatterpfdps, __builtin_ia32_scatterpfqpd,
>         __builtin_ia32_scatterpfqps.
>         (ix86_expand_builtin): Expand new built-ins.
>         * config/i386/sse.md (avx512pf_gatherpf<mode>): Add SF suffix,
>         fix memory access data type.
>         (*avx512pf_gatherpf<mode>_mask): Ditto.
>         (*avx512pf_gatherpf<mode>): Ditto.
>         (avx512pf_scatterpf<mode>): Ditto.
>         (*avx512pf_scatterpf<mode>_mask): Ditto.
>         (*avx512pf_scatterpf<mode>): Ditto.
>         (avx512pf_gatherpf<mode>df): New.
>         (*avx512pf_gatherpf<mode>df_mask): Ditto.
>         (*avx512pf_gatherpf<mode>df): Ditto.
>         (avx512pf_scatterpf<mode>df): Ditto.
>         (*avx512pf_scatterpf<mode>df_mask): Ditto.
>         (*avx512pf_scatterpf<mode>df): Ditto.
>
> testsuite/
>         * gcc.target/i386/avx512f-vmovdqu32-1.c: Fix intrinsic name.
>         * gcc.target/i386/avx512f-vmovdqu32-2.c: Ditto.
>         * gcc.target/i386/avx512f-vpcmpd-2.c: Ditto.
>         * gcc.target/i386/avx512f-vpcmpq-2.c: Ditto.
>         * gcc.target/i386/avx512f-vpcmpud-2.c: Ditto.
>         * gcc.target/i386/avx512f-vpcmpuq-2.c: Ditto.
>         * gcc.target/i386/avx512pf-vgatherpf0dpd-1.c: Ditto.
>         * gcc.target/i386/avx512pf-vgatherpf0qpd-1.c: Ditto.
>         * gcc.target/i386/avx512pf-vgatherpf1dpd-1.c: Ditto.
>         * gcc.target/i386/avx512pf-vgatherpf1qpd-1.c: Ditto.
>         * gcc.target/i386/avx512pf-vscatterpf0dpd-1.c: Ditto.
>         * gcc.target/i386/avx512pf-vscatterpf0qpd-1.c: Ditto.
>         * gcc.target/i386/avx512pf-vscatterpf1dpd-1.c: Ditto.
>         * gcc.target/i386/avx512pf-vscatterpf1qpd-1.c: Ditto.
>         * gcc.target/i386/sse-14.c: Add new built-ins, fix AVX-512ER
>         built-ins roudning immediate.
>         * gcc.target/i386/sse-22.c: Add new built-ins.
>         * gcc.target/i386/sse-23.c: Ditto.
>         * gcc.target/i386/avx-1.c: Ditto.
>
> I have a doubts about changes to sse.md.
> I've splitted existing (SF-only) patterns into 2: DF and SF.
> As far as insn operands and final instruction have no such data
> type discrimination I set this data type to (mem:..) part.
> Having this (for SF):
>   (define_expand "avx512pf_scatterpf<mode>sf"
>     [(unspec
>        [(match_operand:<avx512fmaskmode> 0 "register_or_constm1_operand")
>         (mem:SF
>   ...
>
> instead of this:
>   (define_expand "avx512pf_scatterpf<mode>"
>     [(unspec
>        [(match_operand:<avx512fmaskmode> 0 "register_or_constm1_operand")
>         (mem:<ssescalarmode>
>   ...
>
> Not sure if this (DI/SI) mode for mem is needed. Moreover, not sure what
> that data type represents.

Did you try to add DF/SF mode to the unspec? I am not familiar with
this insn, but shouldn't the mode of mem access be somehow similar to
the avx512f_scattersi<mode> access?

Also, you can use double macroization with MODEF iterator for SF and DFmode.

Uros.