On Tue, 2 Feb 2021, Tamar Christina wrote:

> Hi All,
> 
> Previously the SLP pattern matcher was using STMT_VINFO_SLP_VECT_ONLY as a way
> to dissolve the SLP only patterns during SLP cancellation.  However it seems
> like the semantics for STMT_VINFO_SLP_VECT_ONLY are slightly different than 
> what
> I expected.
> 
> Namely that the non-SLP path can still use a statement marked
> STMT_VINFO_SLP_VECT_ONLY.  One such example is masked loads which are used 
> both
> in the SLP and non-SLP path.
> 
> To fix this I now introduce a new flag STMT_VINFO_SLP_VECT_ONLY_PATTERN which 
> is
> used only by the pattern matcher.
> 
> Bootstrapped Regtested on aarch64-none-linux-gnu, x86_64-pc-linux-gnu
> and no issues.
> 
> Ok for master?

OK.

Thanks,
Richard.

> Thanks,
> Tamar
> 
> gcc/ChangeLog:
> 
>       PR tree-optimization/98928
>       * tree-vect-loop.c (vect_analyze_loop_2): Change
>       STMT_VINFO_SLP_VECT_ONLY to STMT_VINFO_SLP_VECT_ONLY_PATTERN.
>       * tree-vect-slp-patterns.c (complex_pattern::build): Likewise.
>       * tree-vectorizer.h (STMT_VINFO_SLP_VECT_ONLY_PATTERN): New.
>       (class _stmt_vec_info): Add slp_vect_pattern_only_p.
> 
> gcc/testsuite/ChangeLog:
> 
>       PR tree-optimization/98928
>       * gcc.target/i386/pr98928.c: New test.
> 
> --- inline copy of patch -- 
> diff --git a/gcc/testsuite/gcc.target/i386/pr98928.c 
> b/gcc/testsuite/gcc.target/i386/pr98928.c
> new file mode 100644
> index 
> 0000000000000000000000000000000000000000..9503b579a88d95c427d3e3e5a71565b0c048c125
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/i386/pr98928.c
> @@ -0,0 +1,59 @@
> +/* { dg-do compile } */
> +/* { dg-additional-options "-Ofast -march=skylake-avx512 -fwhole-program -w" 
> } */
> +
> +typedef float MagickRealType;
> +typedef short Quantum;
> +float InterpolateMagickPixelPacket_alpha[1];
> +int InterpolateMagickPixelPacket_i;
> +
> +void InterpolateMagickPixelPacket();
> +
> +void main() { InterpolateMagickPixelPacket(); }
> +
> +typedef struct {
> +  MagickRealType red, green, blue, opacity, index;
> +} MagickPixelPacket;
> +typedef struct {
> +  Quantum blue, green, red, opacity;
> +} PixelPacket;
> +struct _Image {
> +  int colorspace;
> +  int matte;
> +} GetMagickPixelPacket(MagickPixelPacket *pixel) {
> +  pixel->red = pixel->green = pixel->blue = 0.0;
> +}
> +int AlphaBlendMagickPixelPacket(struct _Image *image, PixelPacket *color,
> +                            Quantum *indexes, MagickPixelPacket *pixel,
> +                            MagickRealType *alpha) {
> +  if (image->matte) {
> +    *alpha = pixel->red = pixel->green = pixel->blue = pixel->opacity =
> +        color->opacity;
> +    pixel->index = 0.0;
> +    if (image->colorspace)
> +      pixel->index = *indexes;
> +    return 0;
> +  }
> +  *alpha = 1.0 / 0.2;
> +  pixel->red = *alpha * color->red;
> +  pixel->green = *alpha * color->green;
> +  pixel->blue = *alpha * color->blue;
> +  pixel->opacity = pixel->index = 0.0;
> +  if (image->colorspace && indexes)
> +    pixel->index = *indexes;
> +}
> +MagickPixelPacket InterpolateMagickPixelPacket_pixels[1];
> +PixelPacket InterpolateMagickPixelPacket_p;
> +
> +void
> +InterpolateMagickPixelPacket(struct _Image *image) {
> +  Quantum *indexes;
> +  for (; InterpolateMagickPixelPacket_i; InterpolateMagickPixelPacket_i++) {
> +    GetMagickPixelPacket(InterpolateMagickPixelPacket_pixels +
> +                         InterpolateMagickPixelPacket_i);
> +    AlphaBlendMagickPixelPacket(
> +        image, &InterpolateMagickPixelPacket_p + 
> InterpolateMagickPixelPacket_i,
> +        indexes + InterpolateMagickPixelPacket_i,
> +        InterpolateMagickPixelPacket_pixels + InterpolateMagickPixelPacket_i,
> +        InterpolateMagickPixelPacket_alpha + InterpolateMagickPixelPacket_i);
> +  }
> +}
> diff --git a/gcc/tree-vect-loop.c b/gcc/tree-vect-loop.c
> index 
> acfd1952e3b803ea79cf51433101466743c9793e..200ed27b32ef4aa54c6783afa1864924b6f55582
>  100644
> --- a/gcc/tree-vect-loop.c
> +++ b/gcc/tree-vect-loop.c
> @@ -2700,7 +2700,7 @@ again:
>           {
>             stmt_vec_info pattern_stmt_info
>               = STMT_VINFO_RELATED_STMT (stmt_info);
> -           if (STMT_VINFO_SLP_VECT_ONLY (pattern_stmt_info))
> +           if (STMT_VINFO_SLP_VECT_ONLY_PATTERN (pattern_stmt_info))
>               STMT_VINFO_IN_PATTERN_P (stmt_info) = false;
>  
>             gimple *pattern_def_seq = STMT_VINFO_PATTERN_DEF_SEQ (stmt_info);
> diff --git a/gcc/tree-vect-slp-patterns.c b/gcc/tree-vect-slp-patterns.c
> index 
> d25560fab97bb852e949884850d51c6148b14a68..f0817da9f622d22e3df2e30410d1cf610b4ffa1d
>  100644
> --- a/gcc/tree-vect-slp-patterns.c
> +++ b/gcc/tree-vect-slp-patterns.c
> @@ -599,7 +599,7 @@ complex_pattern::build (vec_info *vinfo)
>        the call there.  */
>        vect_mark_pattern_stmts (vinfo, stmt_info, call_stmt,
>                              SLP_TREE_VECTYPE (node));
> -      STMT_VINFO_SLP_VECT_ONLY (call_stmt_info) = true;
> +      STMT_VINFO_SLP_VECT_ONLY_PATTERN (call_stmt_info) = true;
>  
>        /* Since we are replacing all the statements in the group with the same
>        thing it doesn't really matter.  So just set it every time a new stmt
> diff --git a/gcc/tree-vectorizer.h b/gcc/tree-vectorizer.h
> index 
> f8bf4488d0ea32e7909f6be2bf4e7cdaee4f55fe..e564fcf835a46a8c1aa6b5fb52f7ecd60bcb1bc9
>  100644
> --- a/gcc/tree-vectorizer.h
> +++ b/gcc/tree-vectorizer.h
> @@ -1215,6 +1215,10 @@ public:
>  
>    /* True if this is only suitable for SLP vectorization.  */
>    bool slp_vect_only_p;
> +
> +  /* True if this is a pattern that can only be handled by SLP
> +     vectorization.  */
> +  bool slp_vect_pattern_only_p;
>  };
>  
>  /* Information about a gather/scatter call.  */
> @@ -1301,6 +1305,7 @@ struct gather_scatter_info {
>  #define STMT_VINFO_REDUC_VECTYPE(S)     (S)->reduc_vectype
>  #define STMT_VINFO_REDUC_VECTYPE_IN(S)  (S)->reduc_vectype_in
>  #define STMT_VINFO_SLP_VECT_ONLY(S)     (S)->slp_vect_only_p
> +#define STMT_VINFO_SLP_VECT_ONLY_PATTERN(S) (S)->slp_vect_pattern_only_p
>  
>  #define DR_GROUP_FIRST_ELEMENT(S) \
>    (gcc_checking_assert ((S)->dr_aux.dr), (S)->first_element)
> 
> 
> 

-- 
Richard Biener <rguent...@suse.de>
SUSE Software Solutions Germany GmbH, Maxfeldstrasse 5, 90409 Nuernberg,
Germany; GF: Felix Imendörffer; HRB 36809 (AG Nuernberg)

Reply via email to