Re: [PATCH 2/2] Fix PR88784, middle end is missing some optimizations about unsigned

[Richard Biener <rguenther at suse dot de>]

On Fri, 6 Sep 2019, Martin Liška wrote:

> On 9/5/19 3:17 PM, Richard Biener wrote:
> > On Tue, 16 Jul 2019, Li Jia He wrote:
> > 
> >> Hi,
> >>
> >>   I made some changes based on the recommendations. Would you like to
> >>   help me to see it again ? Sorry, it took so long time to provide the
> >>   patch.
> >>
> >>   Note: 1. I keep the code for and_comparisons_1 and or_comparisons_1.
> >> 	The reason is that I did not found a good way to handle the
> >> 	optimization of '((x CODE1 y) AND (x CODE2 y))' in match.pd.
> >> 	Maybe I missing some important information about match.pd.
> >> 	2. The gimple_resimplify2 function is not used.  Since stmt1,
> >> 	stmt2, lhs1 and lhs2 are allocated on the stack, Is there a
> >> 	question with the value on the stack as the return value ?
> >> 	I may have misunderstood Richard's intention.
> > 
> > And now for the match.pd patch.
> > 
> > +/* x >  y  &&  x != XXX_MIN  -->  x > y  */
> > +(for and (truth_and bit_and)
> > + (simplify
> > +  (and:c (gt:c@3 @0 @1) (ne @0 INTEGER_CST@2))
> > +  (if (INTEGRAL_TYPE_P (TREE_TYPE (@0)) && INTEGRAL_TYPE_P 
> > (TREE_TYPE(@1))
> > +       && (wi::eq_p (wi::to_wide (@2), wi::min_value (TREE_TYPE (@2)))))
> > +    @3)))
> > +
> > +/* x >  y  &&  x == XXX_MIN  -->  false  */
> > +(for and (truth_and bit_and)
> > + (simplify
> > +  (and:c (gt:c @0 @1) (eq @0 INTEGER_CST@2))
> > +  (if (INTEGRAL_TYPE_P (TREE_TYPE (@0)) && INTEGRAL_TYPE_P 
> > (TREE_TYPE(@1))
> > +       && (wi::eq_p (wi::to_wide (@2), wi::min_value (TREE_TYPE (@2)))))
> > +    { boolean_false_node; })))
> > 
> > you could merge those two via
> > 
> >  (for eqne (eq ne)
> >   (for and (....
> >    (simplify
> >     (and:c (gt:c @0 @1) (eqne @0 INTEGER_CST@2))
> >     (if (...)
> >      (switch
> >       (if (eqne == NE_EXPR)
> >        @3)
> >       (if (eqne == EQ_EXPR)
> >        { constant_boolean_node (false, type); }))))
> > 
> > notice using constant_boolean_node (false, type); instead of
> > boolean_false_node.  I suspect more unification is possible.
> > 
> > Also you could do
> > 
> > (match min_value
> >  INTEGER_CST
> >  (if (INTEGRAL_TYPE_P (type)
> >       && wi::eq_p (wi::to_wide (t), wi::min_value (type)))))
> > 
> > and then write
> > 
> >  (simplify
> >   (and:c (gt:c @0 @1) (eq @0 min_value))
> >   (...
> > 
> > Your
> > 
> > (if (INTEGRAL_TYPE_P (TREE_TYPE (@0)) && INTEGRAL_TYPE_P
> > (TREE_TYPE(@1))
> > 
> > is redundant, it's enough to check either @0 or @1 given they
> > have to be compatible for the gt operation.  Note you probably
> > want to use
> > 
> >   (and:c (gt:c @0 @1) (eq @@0 min_value))
> > 
> > and verify that types_match (@1, @0) because when @0 are a constant
> > (and (eq @0 min_value) is not folded which can happen) then they
> > might have different types and thus you could have
> > (SHORT_MAX > intvar) && (SHORT_MAX == SHORT_MAX)
> > 
> > That said, the patterns can be quite a bit simplified I think.
> > 
> > Richard.
> > 
> 
> Likewise, I applied the suggested simplification.
> 
> Patch can bootstrap on x86_64-linux-gnu and survives regression tests.
> 
> Ready to be installed?

+/* { dg-options "-O2 -fdump-tree-ifcombine --param 
logical-op-non-short-circuit=0" } */
+
+#include <limits.h>
+
+_Bool and1(unsigned x, unsigned y)
+{
+  /* x > y && x != 0 --> x > y */
+  return x > y && x != 0;
+}
...
+/* { dg-final { scan-tree-dump-not " != " "ifcombine" } } */

since you iterate over bit_and and truth_and GENERIC folding should
already simplify this, no?

I suggest to remove the truth_and/or iteration.

Otherwise looks OK now.

Richard.