On Thu, May 15, 2025 at 5:55 AM Andrew Pinski <pins...@gmail.com> wrote:
>
> On Wed, May 14, 2025 at 7:39 PM Andrew Pinski <quic_apin...@quicinc.com>
> wrote:
> >
> > This is the next step in removing forward_propagate_into_comparison
> > and forward_propagate_into_gimple_cond; In the case of `((int)(a cmp b)) !=
> > 0`
> > we want to do the transformation to `a cmp b` even if the cast is used
> > twice.
> > This is exactly what
> > forward_propagate_into_comparison/forward_propagate_into_gimple_cond
> > do and does the copy.
>
> Actually I am thinking we should change:
> this set of patterns:
> ```
> (for cmp (simple_comparison)
> (simplify
> (cmp (convert@0 @00) (convert?@1 @10))
> (if (INTEGRAL_TYPE_P (TREE_TYPE (@0))
> /* Disable this optimization if we're casting a function pointer
> type on targets that require function pointer canonicalization. */
> && !(targetm.have_canonicalize_funcptr_for_compare ()
> && ((POINTER_TYPE_P (TREE_TYPE (@00))
> && FUNC_OR_METHOD_TYPE_P (TREE_TYPE (TREE_TYPE (@00))))
> || (POINTER_TYPE_P (TREE_TYPE (@10))
> && FUNC_OR_METHOD_TYPE_P (TREE_TYPE (TREE_TYPE (@10))))))
> && single_use (@0))
> ```
> In the case of:
> (if (TREE_CODE (@1) == INTEGER_CST)
> (cmp @00 ...)
> We don't need to care if @0 is single_use or not as we remove one cast.
> Plus all of the cases where we produce constants don't care about
> single_use either.
>
> So let's ignore this patch for now. I will get back to it tomorrow.
I have a recollection that I changed exactly one similar place to defer
the single_use () checks to the result expressions that are not "simple" ...
IIRC I had that changed :S flag handling for this - enforce single-use
when the result isn't "simple" (much similar to how we have ! now),
but IIRC we concluded we don't need this.
PR118483 was the PR where this mattered, the match.pd change I
had for this was the following:
diff --git a/gcc/match.pd b/gcc/match.pd
index f4050687647..8ad574e7404 100644
--- a/gcc/match.pd
+++ b/gcc/match.pd
@@ -7562,7 +7562,7 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
FIXME: the lack of symmetry is disturbing. */
(for cmp (simple_comparison)
(simplify
- (cmp (convert@0 @00) (convert?@1 @10))
+ (cmp (convert:S@0 @00) (convert?@1 @10))
(if (INTEGRAL_TYPE_P (TREE_TYPE (@0))
/* Disable this optimization if we're casting a function pointer
type on targets that require function pointer canonicalization. */
@@ -7570,8 +7570,7 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
&& ((POINTER_TYPE_P (TREE_TYPE (@00))
&& FUNC_OR_METHOD_TYPE_P (TREE_TYPE (TREE_TYPE (@00))))
|| (POINTER_TYPE_P (TREE_TYPE (@10))
- && FUNC_OR_METHOD_TYPE_P (TREE_TYPE (TREE_TYPE (@10))))))
- && single_use (@0))
+ && FUNC_OR_METHOD_TYPE_P (TREE_TYPE (TREE_TYPE (@10)))))))
(if (TYPE_PRECISION (TREE_TYPE (@00)) == TYPE_PRECISION (TREE_TYPE (@0))
&& (TREE_CODE (@10) == INTEGER_CST
|| @1 != @10)
because the
(if (above || below)
(if (cmp == EQ_EXPR || cmp == NE_EXPR)
{ constant_boolean_node (cmp == EQ_EXPR ? false : true, type); }
(if (cmp == LT_EXPR || cmp == LE_EXPR)
{ constant_boolean_node (above ? true : false, type); }
(if (cmp == GT_EXPR || cmp == GE_EXPR)
{ constant_boolean_node (above ? false : true, type); })))))))))
cases at least would be unproblematic and OK when !single_use. That's exactly
what you noticed. So moving that check down to where it matters works as well
and is pre-approved.
Richard.
>
> Thanks,
> Andrew
>
> >
> > Bootstrapped and tested on x86_64-linux-gnu.
> >
> > gcc/ChangeLog:
> >
> > * match.pd (`(a cmp b) != false`, `(a cmp b) == true`,
> > `(a cmp b) != true`, `(a cmp b) == false`): Allow an
> > optional cast between the comparison and the eq/ne.
> > (`bool_val != false`, `bool_val == true`): Allow an optional
> > cast between the bool_val and the ne/eq.
> >
> > Signed-off-by: Andrew Pinski <quic_apin...@quicinc.com>
> > ---
> > gcc/match.pd | 12 ++++++------
> > 1 file changed, 6 insertions(+), 6 deletions(-)
> >
> > diff --git a/gcc/match.pd b/gcc/match.pd
> > index 79485f9678a..ffb1695e6e6 100644
> > --- a/gcc/match.pd
> > +++ b/gcc/match.pd
> > @@ -6913,15 +6913,15 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
> > (ncmp @0 @1)))))
> > /* The following bits are handled by fold_binary_op_with_conditional_arg.
> > */
> > (simplify
> > - (ne (cmp@2 @0 @1) integer_zerop)
> > + (ne (convert? (cmp@2 @0 @1)) integer_zerop)
> > (if (types_match (type, TREE_TYPE (@2)))
> > (cmp @0 @1)))
> > (simplify
> > - (eq (cmp@2 @0 @1) integer_truep)
> > + (eq (convert? (cmp@2 @0 @1)) integer_truep)
> > (if (types_match (type, TREE_TYPE (@2)))
> > (cmp @0 @1)))
> > (simplify
> > - (ne (cmp@2 @0 @1) integer_truep)
> > + (ne (convert? (cmp@2 @0 @1)) integer_truep)
> > (if (types_match (type, TREE_TYPE (@2)))
> > (with { enum tree_code ic = invert_tree_comparison
> > (cmp, HONOR_NANS (@0)); }
> > @@ -6930,7 +6930,7 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
> > (if (ic == ncmp)
> > (ncmp @0 @1))))))
> > (simplify
> > - (eq (cmp@2 @0 @1) integer_zerop)
> > + (eq (convert? (cmp@2 @0 @1)) integer_zerop)
> > (if (types_match (type, TREE_TYPE (@2)))
> > (with { enum tree_code ic = invert_tree_comparison
> > (cmp, HONOR_NANS (@0)); }
> > @@ -8104,13 +8104,13 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
> >
> > /* bool_var != 0 becomes bool_var. */
> > (simplify
> > - (ne @0 integer_zerop)
> > + (ne (convert? @0) integer_zerop)
> > (if (TREE_CODE (TREE_TYPE (@0)) == BOOLEAN_TYPE
> > && types_match (type, TREE_TYPE (@0)))
> > (non_lvalue @0)))
> > /* bool_var == 1 becomes bool_var. */
> > (simplify
> > - (eq @0 integer_onep)
> > + (eq (convert? @0) integer_onep)
> > (if (TREE_CODE (TREE_TYPE (@0)) == BOOLEAN_TYPE
> > && types_match (type, TREE_TYPE (@0)))
> > (non_lvalue @0)))
> > --
> > 2.43.0
> >
