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.
