On Thu, 20 Aug 2026, Victor Do Nascimento wrote:

> Richard Biener <[email protected]> writes:
> 
> > On Mon, 17 Aug 2026, Victor Do Nascimento wrote:
> >
> >> Consider the following dot-product computation:
> >
> > I'm a bit confused about the back-to-back of the reply to v2
> > and this unversioned patch?  Is this one v3?
> >
> > Please state which patch you consider latest and want reviewed.
> >
> > Thanks,
> > Richard.
> >
> 
> You're right, the way I structured things wasn't the most clear.
> 
> The way the scope of the patch ultimately expanded meant that this
> version got a (slightly) new title reflecting this.  I wasn't quite sure
> whether [PATCH v3] made sense when the titles, and thus what the patch
> does, no longer matched exactly.
> 
> Thus, having mentioned in my own v2 review that I'd modify the patch
> to take into consideration the SAD_EXPR and that I'd submit it in a 
> new patch, I made this its own new "v1" instead.
> 
> Perhaps, once this was submitted, I should have posted a link to it in
> the previous version.
> 
> Anyway, sorry for the confusion.
> 
> This is the patch that needs reviewing.

I'll note that with looking at SSA use->def chains in
same_mask_and_else_value_p you are overriding a valueize hooks
preference to not do this.  match patterns use get_def ()
for this (but it's private to gimple-match-*)
The pattern should be #if GIMPLE only for the same reason.

While the match infrastructure knows mask and else masked
operations in a general way this isn't reflected in the language,
so you can't say

(simplify
 (vec_cond @mask (masked@0 @mask @else @...) @else)
 @0)

but you would need to enumerate all masked functions.  See
gimple-match-exports.cc:maybe_resimplify_conditional_op.

At least using gimple_extract_op could simplify your
function, making mask and else value of a gimple * accessible,
possibly allowing a

(match (conditional_op @mask @else)
 SSA_NAME@0
 (with
  {
    gimple_match_op op;
  }
  (if (gimple_extract_op (SSA_NAME_DEF_STMT (@0), &op)
       && op.cond.cond && op.cond.else_value)
   (with
     {
       /* Ick.  */
       @mask = op.cond.cond;
       @else = op.cond.else;
     }))))

But a "built-in" (match (conditional_op ...)) could possibly be
invented (not easily a way to extract the unconditional op though(?)).

So you could in the end write

(simplify
 (vec_cond @mask (conditional_op@0 @mask @else) @else)
 @0)

Thanks,
Richard.

> Thanks,
> Victor
> 
> >>   uint32_t
> >>   tcp_checksum(int n, uint8_t* data)
> >>   {
> >>     uint32_t sum = 0;
> >>     for (int i=0; i<n; i+=1)
> >>       sum += data[i] * data[i];
> >>     return sum;
> >>   }
> >> 
> >> At present, following vectorization and the subsequent optimization
> >> passes, we will end up with the following GIMPLE code:
> >> 
> >>   vect__1 = .MASK_LOAD (vectp_data, 8B, loop_mask_1, { 0, ... });
> >>   masked_op1_1 = VEC_COND_EXPR <loop_mask_1, vect__1, { 0, ... }>;
> >>   vect_patt_1 = DOT_PROD_EXPR <vect__1, masked_op1_1, vect_sum_1>;
> >> 
> >> While in this case the `VEC_COND_EXPR' is redundant, we cannot make the
> >> assumption that input data for the vectorized dot product would always
> >> already be masked.  As such, it is correct that `VEC_COND_EXPR'
> >> should be emitted in conjunction with `DOT_PROD_EXPR' by the vectorizer
> >> in order to emulate masked dot product operations.
> >> 
> >> Where simplification is possible, a better approach that maintains
> >> correctness is to look at the input data, mask and else value going into
> >> the VEC_COND_EXPR, walking back up the USE-DEF chain to see whether the
> >> source of the input data shares the same mask and else values.  If so,
> >> we can safely remove the VEC_COND_EXPR statement from  the cfg, thus
> >> resulting in the more optimal variant:
> >> 
> >>   vect__1 = .MASK_LOAD (vectp_data, 8B, loop_mask_1, { 0, ... });
> >>   vect_patt_1 = DOT_PROD_EXPR <vect__1, vect__1, vect_sum_1>;
> >> 
> >> Another, more complex, example of where the VEC_COND_EXPR removal is
> >> valid comes from the sum-of-absolute-differences computation, wherein
> >> we see:
> >> 
> >>   vect__1 = .MASK_LOAD (_220, 8B, loop_mask_1, { 0, ... });
> >>   vect__2 = .MASK_LOAD (_232, 8B, loop_mask_1, { 0, ... });
> >>   masked_op1_1 =  VEC_COND_EXPR (loop_mask_1, vect__2, vect__1);
> >>   vect_patt_1 = SAD_EXPR <vect__1, masked_op1_1, vect_result_1>;
> >> 
> >> Here, comparing the mask and else values of `masked_op1_1' and `vect__2'
> >> gives:
> >> 
> >>   (mask) loop_mask_1 == loop_mask_1
> >>   (else) vect__1 != { 0. ... }
> >> 
> >> which would fail the original condition stipulated above.
> >> 
> >> It is worth noting, however, that we don't care about the entire
> >> contents of the "else" vector, only the values it provides for the
> >> VEC_COND_EXPR inactive lanes.  This allows for a further refinement of
> >> the simplification condition.  If the else vector is itself an SSA
> >> definition, populated using the same mask and the same else values,
> >> then the equality condition required for safe removal of the
> >> VEC_COND_EXPR is still valid, e.g.
> >> 
> >>   (mask) loop_mask_1 == loop_mask_1
> >>   (else) (vect__1 & loop_mask_1) == { 0. ... }
> >> 
> >> This allows us to implement the necessary checks recursively and pick
> >> up more simplification cases.
> >> 
> >> We can do this in `match.pd' via a simple pattern, e.g.
> >> 
> >> (simplify
> >>  (vec_cond @0 @1 @2)
> >>  (if (same_mask_and_else_value_p (@0, @1, @2))
> >>   @1))
> >> 
> >> and implement it in the current patch.
> >> 
> >>    PR tree-optimization/111770
> >> 
> >> gcc/Changelog:
> >> 
> >>    * match.pd: Add `same_mask_and_else_value_p' to
> >>    `define_predicates', applying it to `vec_cond' expression
> >>    simplification.
> >>    * tree.cc (same_mask_and_else_value_p): New.
> >>    * tree.h (same_mask_and_else_value_p): Likewise.
> >> 
> >> gcc/testsuite/ChangeLog:
> >> 
> >>    * gcc.dg/vect/vect-cond-dot.c: New
> >>    * gcc.dg/vect/vect-cond-sad.c: New
> >> ---
> >>  gcc/match.pd                              | 11 +++++-
> >>  gcc/testsuite/gcc.dg/vect/vect-cond-dot.c | 20 +++++++++++
> >>  gcc/testsuite/gcc.dg/vect/vect-cond-sad.c | 24 +++++++++++++
> >>  gcc/tree.cc                               | 42 +++++++++++++++++++++++
> >>  gcc/tree.h                                |  1 +
> >>  5 files changed, 97 insertions(+), 1 deletion(-)
> >>  create mode 100644 gcc/testsuite/gcc.dg/vect/vect-cond-dot.c
> >>  create mode 100644 gcc/testsuite/gcc.dg/vect/vect-cond-sad.c
> >> 
> >> diff --git a/gcc/match.pd b/gcc/match.pd
> >> index 0ba97b32cb1..3da40f6adcb 100644
> >> --- a/gcc/match.pd
> >> +++ b/gcc/match.pd
> >> @@ -40,7 +40,8 @@ along with GCC; see the file COPYING3.  If not see
> >>     HONOR_NANS
> >>     uniform_vector_p
> >>     expand_vec_cmp_expr_p
> >> -   bitmask_inv_cst_vector_p)
> >> +   bitmask_inv_cst_vector_p
> >> +   same_mask_and_else_value_p)
> >>  
> >>  /* Operator lists.  */
> >>  (define_operator_list tcc_comparison
> >> @@ -8897,6 +8898,14 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
> >>     (icmp @0 { csts; })
> >>     (icmp (view_convert:utype @0) { csts; })))))))))
> >>  
> >> +/* Transform cases where VEC_COND_EXPR carries out a redundant operation,
> >> +   e.g. masking out values that have already been masked out from a 
> >> previous
> >> +   masking operation such as a masked load.  */
> >> +(simplify
> >> + (vec_cond @0 @1 @2)
> >> + (if (same_mask_and_else_value_p (@0, @1, @2))
> >> +  @1))
> >> +
> >>  /* When one argument is a constant, overflow detection can be simplified.
> >>     Currently restricted to single use so as not to interfere too much with
> >>     ADD_OVERFLOW detection in tree-ssa-math-opts.cc.
> >> diff --git a/gcc/testsuite/gcc.dg/vect/vect-cond-dot.c 
> >> b/gcc/testsuite/gcc.dg/vect/vect-cond-dot.c
> >> new file mode 100644
> >> index 00000000000..667519d1e50
> >> --- /dev/null
> >> +++ b/gcc/testsuite/gcc.dg/vect/vect-cond-dot.c
> >> @@ -0,0 +1,20 @@
> >> +/* { dg-do compile } */
> >> +/* { dg-additional-options "-fdump-tree-optimized" } */
> >> +/* { dg-require-effective-target vect_masked_load } */
> >> +#include <stdint.h>
> >> +
> >> +#define CHECK_DOT(IN, OUT)                \
> >> +OUT check_dot_##OUT(int n, IN* data) {    \
> >> +  OUT sum = 0;                            \
> >> +  for (int i=0; i<n; i+=1) {              \
> >> +    sum += data[i] * data[i];             \
> >> +  }                                       \
> >> +  return sum;                             \
> >> +}
> >> +
> >> +CHECK_DOT (uint8_t, uint32_t);
> >> +CHECK_DOT (int8_t, int32_t);
> >> +CHECK_DOT (int16_t, int64_t);
> >> +
> >> +/* { dg-final { scan-tree-dump-times {vectorized 1 loops} 3 "vect"  } } */
> >> +/* { dg-final { scan-tree-dump-not {VEC_COND_EXPR} "optimized" } } */
> >> diff --git a/gcc/testsuite/gcc.dg/vect/vect-cond-sad.c 
> >> b/gcc/testsuite/gcc.dg/vect/vect-cond-sad.c
> >> new file mode 100644
> >> index 00000000000..a5fa1f50838
> >> --- /dev/null
> >> +++ b/gcc/testsuite/gcc.dg/vect/vect-cond-sad.c
> >> @@ -0,0 +1,24 @@
> >> +/* { dg-do compile } */
> >> +/* { dg-additional-options "-fdump-tree-optimized" } */
> >> +/* { dg-require-effective-target vect_masked_load } */
> >> +
> >> +#define N 64
> >> +
> >> +unsigned char X[N] __attribute__ ((__aligned__(__BIGGEST_ALIGNMENT__)));
> >> +unsigned char Y[N] __attribute__ ((__aligned__(__BIGGEST_ALIGNMENT__)));
> >> +int abs (int);
> >> +
> >> +__attribute__ ((noinline)) int
> >> +foo (int len)
> >> +{
> >> +  int i;
> >> +  int result = 0;
> >> +
> >> +  for (i = 0; i < len; i++)
> >> +    result += abs (X[i] - Y[i]);
> >> +
> >> +  return result;
> >> +}
> >> +
> >> +/* { dg-final { scan-tree-dump {vectorized 1 loops} "vect"  } } */
> >> +/* { dg-final { scan-tree-dump-not {VEC_COND_EXPR} "optimized" } } */
> >> diff --git a/gcc/tree.cc b/gcc/tree.cc
> >> index c8aa42b3e10..93973aa8a4c 100644
> >> --- a/gcc/tree.cc
> >> +++ b/gcc/tree.cc
> >> @@ -12343,6 +12343,48 @@ block_ultimate_origin (const_tree block)
> >>      }
> >>  }
> >>  
> >> +/* Look for masking redundancy.  When applying a mask, check whether the
> >> +   statement defining the input values uses the same mask and else values 
> >> as the
> >> +   current masking operation, in which case the masking operation is 
> >> redundant
> >> +   and may be safely eliminated.
> >> +
> >> +   The functionality is recursive.  If the else values don't match, but 
> >> ELSE_VAL
> >> +   is itself a SSA assignment from a masked operation, and that has the 
> >> same
> >> +   inactive lanes with the same values, the overall result is the same.  
> >> */
> >> +
> >> +bool
> >> +same_mask_and_else_value_p (tree mask, tree then_val, tree else_val)
> >> +{
> >> +  if (then_val && TREE_CODE (then_val) == SSA_NAME)
> >> +    {
> >> +      /* Walk back up the use-def chain and see whether value comes from a
> >> +   masked operation.  */
> >> +      gimple *then_defn = SSA_NAME_DEF_STMT (then_val);
> >> +      if (then_defn
> >> +    && is_gimple_call (then_defn)
> >> +    && gimple_call_internal_p (then_defn))
> >> +  {
> >> +    internal_fn ifn = gimple_call_internal_fn (then_defn);
> >> +    int mask_index = internal_fn_mask_index (ifn);
> >> +    if (mask_index == 0)
> >> +      return false;
> >> +
> >> +    /* See how the defining masked operation populated inactive lanes
> >> +       and compare this to how the current masked op handles its
> >> +       inactive lanes.  */
> >> +    int false_index = internal_fn_else_index (ifn);
> >> +    tree then_mask = gimple_call_arg (then_defn, mask_index);
> >> +    tree then_false = gimple_call_arg (then_defn, false_index);
> >> +
> >> +    if (mask == then_mask)
> >> +      if (operand_equal_p (else_val, then_false, 0)
> >> +          || same_mask_and_else_value_p (then_mask, else_val, then_false))
> >> +        return true;
> >> +  }
> >> +    }
> >> +  return false;
> >> +}
> >> +
> >>  /* Return true iff conversion from INNER_TYPE to OUTER_TYPE generates
> >>     no instruction.  */
> >>  
> >> diff --git a/gcc/tree.h b/gcc/tree.h
> >> index 1ccbf848d9b..68adddcd5f6 100644
> >> --- a/gcc/tree.h
> >> +++ b/gcc/tree.h
> >> @@ -5783,6 +5783,7 @@ extern bool prototype_p (const_tree);
> >>  extern bool auto_var_p (const_tree);
> >>  extern bool auto_var_in_fn_p (const_tree, const_tree);
> >>  extern tree build_low_bits_mask (tree, unsigned);
> >> +extern bool same_mask_and_else_value_p (tree, tree, tree);
> >>  extern bool tree_nop_conversion_p (const_tree, const_tree);
> >>  extern tree tree_strip_nop_conversions (tree);
> >>  extern tree tree_strip_sign_nop_conversions (tree);
> >> 
> 

-- 
Richard Biener <[email protected]>
SUSE Software Solutions Germany GmbH,
Frankenstrasse 146, 90461 Nuernberg, Germany;
GF: Jochen Jaser, Andrew McDonald, Abhinav Puri; (HRB 36809, AG Nuernberg)

Reply via email to