Re: [PATCH] [AVX512F] Add scatter support for vectorizer

[Uros Bizjak <ubizjak at gmail dot com>]

On Wed, Aug 26, 2015 at 7:39 PM, Petr Murzin <petrmurzin1@gmail.com> wrote:
> On Wed, Aug 26, 2015 at 10:41 AM, Richard Biener
> <richard.guenther@gmail.com> wrote:
>> @@ -3763,32 +3776,46 @@ again:
>>        if (vf > *min_vf)
>>         *min_vf = vf;
>>
>> -      if (gather)
>> +      if (gatherscatter != SG_NONE)
>>         {
>>           tree off;
>> +         if (vect_check_gather_scatter (stmt, loop_vinfo, NULL, &off,
>> NULL, true) != 0)
>> +           gatherscatter = GATHER;
>> +         else if (vect_check_gather_scatter (stmt, loop_vinfo, NULL,
>> &off, NULL, false)
>> +                                             != 0)
>> +           gatherscatter = SCATTER;
>> +         else
>> +           gatherscatter = SG_NONE;
>>
>> as I said vect_check_gather_scatter already knows whether the DR is a read or
>> a write and thus whether it needs to check for gather or scatter.  Remove
>> the new argument.  And simply do
>>
>>    if (!vect_check_gather_scatter (stmt....))
>>      gatherscatter = SG_NONE;
>>
>> -         STMT_VINFO_GATHER_P (stmt_info) = true;
>> +         if (gatherscatter == GATHER)
>> +           STMT_VINFO_GATHER_P (stmt_info) = true;
>> +         else
>> +           STMT_VINFO_SCATTER_P (stmt_info) = true;
>>         }
>>
>> and as suggested merge STMT_VINFO_GATHER_P and STMT_VINFO_SCATTER_P
>> using the enum so you can simply do
>>
>>  STMT_VINFO_SCATTER_GATHER_P (smt_info) = gatherscatter;
>> Otherwise the patch looks ok to me.
>
> Fixed.
> Uros, could you please have a look at target part of patch?
>
> 2015-08-26  Andrey Turetskiy  <andrey.turetskiy@intel.com>
>             Petr Murzin  <petr.murzin@intel.com>
>
> gcc/
>
> * config/i386/i386-builtin-types.def
> (VOID_PFLOAT_HI_V8DI_V16SF_INT): New.
> (VOID_PDOUBLE_QI_V16SI_V8DF_INT): Ditto.
> (VOID_PINT_HI_V8DI_V16SI_INT): Ditto.
> (VOID_PLONGLONG_QI_V16SI_V8DI_INT): Ditto.
> * config/i386/i386.c
> (ix86_builtins): Add IX86_BUILTIN_SCATTERALTSIV8DF,
> IX86_BUILTIN_SCATTERALTDIV16SF, IX86_BUILTIN_SCATTERALTSIV8DI,
> IX86_BUILTIN_SCATTERALTDIV16SI.
> (ix86_init_mmx_sse_builtins): Define __builtin_ia32_scatteraltsiv8df,
> __builtin_ia32_scatteraltdiv8sf, __builtin_ia32_scatteraltsiv8di,
> __builtin_ia32_scatteraltdiv8si.
> (ix86_expand_builtin): Handle IX86_BUILTIN_SCATTERALTSIV8DF,
> IX86_BUILTIN_SCATTERALTDIV16SF, IX86_BUILTIN_SCATTERALTSIV8DI,
> IX86_BUILTIN_SCATTERALTDIV16SI.
> (ix86_vectorize_builtin_scatter): New.
> (TARGET_VECTORIZE_BUILTIN_SCATTER): Define as
> ix86_vectorize_builtin_scatter.
> * doc/tm.texi.in (TARGET_VECTORIZE_BUILTIN_SCATTER): New.
> * doc/tm.texi: Regenerate.
> * target.def: Add scatter builtin.
> * tree-vectorizer.h: Rename gather_p to gather_scatter_p and use it
> for loads/stores in case of gather/scatter accordingly.
> (STMT_VINFO_GATHER_SCATTER_P(S)): Use it instead of STMT_VINFO_GATHER_P(S).
> (vect_check_gather): Rename to ...
> (vect_check_gather_scatter): this.
> * tree-vect-data-refs.c (vect_analyze_data_ref_dependence): Use
> STMT_VINFO_GATHER_SCATTER_P instead of STMT_VINFO_SCATTER_P.
> (vect_check_gather_scatter): Use it instead of vect_check_gather.
> (vect_analyze_data_refs): Add gatherscatter enum and maybe_scatter variable
> and new checkings for it accordingly.
> * tree-vect-stmts.c
> (STMT_VINFO_GATHER_SCATTER_P(S)): Use it instead of STMT_VINFO_GATHER_P(S).
> (vect_check_gather_scatter): Use it instead of vect_check_gather.
> (vectorizable_store): Add checkings for STMT_VINFO_GATHER_SCATTER_P.
>
> gcc/testsuite/
>
>         * gcc.target/i386/avx512f-scatter-1.c: New.
>         * gcc.target/i386/avx512f-scatter-2.c: Ditto.
>         * gcc.target/i386/avx512f-scatter-3.c: Ditto.

x86 target part and testsuite are OK with the following change to the testcases:

> +/* { dg-do run } */
> +/* { dg-require-effective-target avx512f } */
> +/* { dg-options "-O3 -mavx512f -DAVX512F" } */
> +
> +#include "avx512f-check.h"
> +
> +#define N 1024

We don't want -D in the options, please move these to the source:

/* { dg-do run } */
/* { dg-require-effective-target avx512f } */
/* { dg-options "-O3 -mavx512f" } */

#define AVX512F

#include "avx512f-check.h"

#define N 1024

Thanks,
Uros.