Re: [PATCH] tree-opt: Remove redundant VEC_COND_EXPR statements involving masked inputs [PR111770]
Richard Biener <[email protected]>
| Newsgroups | gmane.comp.gcc.patches |
|---|---|
| Message-ID | <[email protected]> |
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)