fold: Allow SSA names in inverse_conditions_p and fold VCOND_MASK.

Hi,

In preparation for the maskload else operand I split off this patch.  The patch
looks through SSA names for the conditions passed to inverse_conditions_p which
helps match.pd recognize more redundant vec_cond expressions.  It also adds
VCOND_MASK to the respective iterators in match.pd.

Is this acceptable without a separate test?  There will of course be several
hits once we emit VEC_COND_EXPRs after masked loads.

The initial version of the patch looked "through" each condition individually.
That caused the following problem on p10 during phiopt:

 foo = blah <= 0
 cond2: foo ? c : x
 cond1: blah > 0 ? b : cond1
 -> (match.pd:6205)
 res = blah > 0 ? b : c
 which is invalid gimple (as blah > 0 is directly used and not put in
 a variable).

Therefore, for now, I restricted the SSA_NAME check to both conditions
simultaneously so we don't run into this situation.  There must be a better
way, though?

Bootstrapped and regtested on x86, aarch64 and power10.
Regtested on armv8.8-a+sve using qemu as well as riscv64.

Regards
 Robin

gcc/ChangeLog:

	* fold-const.cc (inverse_conditions_p): Look through SSA names.
	* match.pd: Add VCOND_MASK to "cond" iterators.
---
 gcc/fold-const.cc | 22 ++++++++++++++++++++++
 gcc/match.pd      | 28 +++++++++++++++-------------
 2 files changed, 37 insertions(+), 13 deletions(-)

On Thu, Jul 25, 2024 at 3:34 PM Robin Dapp <rdapp.gcc@gmail.com> wrote:
>
> Hi,
>
> In preparation for the maskload else operand I split off this patch.  The patch
> looks through SSA names for the conditions passed to inverse_conditions_p which
> helps match.pd recognize more redundant vec_cond expressions.  It also adds
> VCOND_MASK to the respective iterators in match.pd.

IFN_VCOND_MASK should only appear after ISEL so I fail to see why we need
this?  Why are the VEC_CONDs not folded earlier?

> Is this acceptable without a separate test?  There will of course be several
> hits once we emit VEC_COND_EXPRs after masked loads.
>
> The initial version of the patch looked "through" each condition individually.
> That caused the following problem on p10 during phiopt:
>
>  foo = blah <= 0
>  cond2: foo ? c : x
>  cond1: blah > 0 ? b : cond1
>  -> (match.pd:6205)
>  res = blah > 0 ? b : c
>  which is invalid gimple (as blah > 0 is directly used and not put in
>  a variable).

but cond1: blah > 0 ? b : cond1 is already invalid GIMPLE?

Was this supposed to hit only on transitional pattern stmts?!  I don't see
how

(for cond_op (COND_BINARY)
 (simplify
  (vec_cond @0 (view_convert? (cond_op @0 @1 @2 @3)) @4)
  (with { tree op_type = TREE_TYPE (@3); }
   (if (element_precision (type) == element_precision (op_type))
    (view_convert (cond_op @0 @1 @2 (view_convert:op_type @4))))))
 (simplify
  (vec_cond @0 @1 (view_convert? (cond_op @2 @3 @4 @5)))
  (with { tree op_type = TREE_TYPE (@5); }
   (if (inverse_conditions_p (@0, @2)
        && element_precision (type) == element_precision (op_type))
    (view_convert (cond_op @2 @3 @4 (view_convert:op_type @1)))))))

ever can match at the moment (on valid GIMPLE).

I'd open-code inverse_conditions_p like

(for cmp (tcc_comparison)
      icmp (inverted_tcc_comparison)
 (vec_cond (cmp@0 @00 @01) @1 (view_convert? (cond_op (icmp@2 @20 @21
@3 @4 @5)))
 (if (invert_tree_comparison (cmp, HONOR_NANS (@00)) == icmp)
  ...

that's the way other patterns do it.

Richard.

> Therefore, for now, I restricted the SSA_NAME check to both conditions
> simultaneously so we don't run into this situation.  There must be a better
> way, though?
>
> Bootstrapped and regtested on x86, aarch64 and power10.
> Regtested on armv8.8-a+sve using qemu as well as riscv64.
>
> Regards
>  Robin
>
> gcc/ChangeLog:
>
>         * fold-const.cc (inverse_conditions_p): Look through SSA names.
>         * match.pd: Add VCOND_MASK to "cond" iterators.
> ---
>  gcc/fold-const.cc | 22 ++++++++++++++++++++++
>  gcc/match.pd      | 28 +++++++++++++++-------------
>  2 files changed, 37 insertions(+), 13 deletions(-)
>
> diff --git a/gcc/fold-const.cc b/gcc/fold-const.cc
> index 83c32dd10d4..1fc5d97dccc 100644
> --- a/gcc/fold-const.cc
> +++ b/gcc/fold-const.cc
> @@ -86,6 +86,7 @@ along with GCC; see the file COPYING3.  If not see
>  #include "vec-perm-indices.h"
>  #include "asan.h"
>  #include "gimple-range.h"
> +#include "cfgexpand.h"
>
>  /* Nonzero if we are folding constants inside an initializer or a C++
>     manifestly-constant-evaluated context; zero otherwise.
> @@ -3010,6 +3011,27 @@ compcode_to_comparison (enum comparison_code code)
>  bool
>  inverse_conditions_p (const_tree cond1, const_tree cond2)
>  {
> +  /* If both conditions are SSA names, look through them.
> +     Right now callees in match use one of the conditions directly and
> +     we might end up having one in a COND_EXPR like
> +       res = a > b ? c : d
> +     instead of
> +       cnd = a > b
> +       res = cnd ? c : d.
> +
> +     Therefore always consider both conditions simultaneously.  */
> +  if (TREE_CODE (cond1) == SSA_NAME
> +      && TREE_CODE (cond2) == SSA_NAME)
> +    {
> +      gimple *gcond1 = SSA_NAME_DEF_STMT (cond1);
> +      if (is_gimple_assign (gcond1))
> +       cond1 = gimple_assign_rhs_to_tree (gcond1);
> +
> +      gimple *gcond2 = SSA_NAME_DEF_STMT (cond2);
> +      if (is_gimple_assign (gcond2))
> +       cond2 = gimple_assign_rhs_to_tree (gcond2);
> +    }
> +
>    return (COMPARISON_CLASS_P (cond1)
>           && COMPARISON_CLASS_P (cond2)
>           && (invert_tree_comparison
> diff --git a/gcc/match.pd b/gcc/match.pd
> index cf359b0ec0f..f244e6deff5 100644
> --- a/gcc/match.pd
> +++ b/gcc/match.pd
> @@ -5601,7 +5601,7 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
>  /* (a ? x : y) == (b ? x : y) --> (a^b) ? FALSE : TRUE  */
>  /* (a ? x : y) != (b ? y : x) --> (a^b) ? FALSE : TRUE  */
>  /* (a ? x : y) == (b ? y : x) --> (a^b) ? TRUE  : FALSE */
> -(for cnd (cond vec_cond)
> +(for cnd (cond vec_cond IFN_VCOND_MASK)
>   (for eqne (eq ne)
>    (simplify
>     (eqne:c (cnd @0 @1 @2) (cnd @3 @1 @2))
> @@ -5614,14 +5614,15 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
>
>  /* Canonicalize mask ? { 0, ... } : { -1, ...} to ~mask if the mask
>     types are compatible.  */
> -(simplify
> - (vec_cond @0 VECTOR_CST@1 VECTOR_CST@2)
> - (if (VECTOR_BOOLEAN_TYPE_P (type)
> -      && types_match (type, TREE_TYPE (@0)))
> -  (if (integer_zerop (@1) && integer_all_onesp (@2))
> -   (bit_not @0)
> -   (if (integer_all_onesp (@1) && integer_zerop (@2))
> -    @0))))
> +(for cnd (vec_cond IFN_VCOND_MASK)
> + (simplify
> +  (cnd @0 VECTOR_CST@1 VECTOR_CST@2)
> +  (if (VECTOR_BOOLEAN_TYPE_P (type)
> +       && types_match (type, TREE_TYPE (@0)))
> +   (if (integer_zerop (@1) && integer_all_onesp (@2))
> +    (bit_not @0)
> +    (if (integer_all_onesp (@1) && integer_zerop (@2))
> +     @0)))))
>
>  /* A few simplifications of "a ? CST1 : CST2". */
>  /* NOTE: Only do this on gimple as the if-chain-to-switch
> @@ -6049,7 +6050,7 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
>                 { build_int_cst (integer_type_node, prec - 1);}))))))
>  #endif
>
> -(for cnd (cond vec_cond)
> +(for cnd (cond vec_cond IFN_VCOND_MASK)
>   /* (a != b) ? (a - b) : 0 -> (a - b) */
>   (simplify
>    (cnd (ne:c @0 @1) (minus@2 @0 @1) integer_zerop)
> @@ -6185,7 +6186,7 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
>   (if (TYPE_UNSIGNED (type))
>    (cond (ge @0 @1) (negate @0) @2)))
>
> -(for cnd (cond vec_cond)
> +(for cnd (cond vec_cond IFN_VCOND_MASK)
>   /* A ? B : (A ? X : C) -> A ? B : C.  */
>   (simplify
>    (cnd @0 (cnd @0 @1 @2) @3)
> @@ -6210,8 +6211,9 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
>   /* A ? B : B -> B.  */
>   (simplify
>    (cnd @0 @1 @1)
> -  @1)
> +  @1))
>
> +(for cnd (cond vec_cond)
>   /* !A ? B : C -> A ? C : B.  */
>   (simplify
>    (cnd (logical_inverted_value truth_valued_p@0) @1 @2)
> @@ -6232,7 +6234,7 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
>     Note that all these transformations are correct if A is
>     NaN, since the two alternatives (A and -A) are also NaNs.  */
>
> -(for cnd (cond vec_cond)
> +(for cnd (cond vec_cond IFN_VCOND_MASK)
>   /* A == 0 ? A : -A    same as -A */
>   (for cmp (eq uneq)
>    (simplify
> --
> 2.45.2
>