This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH][match-and-simplify] Fix comparison pattern
- From: Richard Biener <rguenther at suse dot de>
- To: gcc-patches at gcc dot gnu dot org
- Date: Wed, 20 Aug 2014 15:54:15 +0200 (CEST)
- Subject: Re: [PATCH][match-and-simplify] Fix comparison pattern
- Authentication-results: sourceware.org; auth=none
- References: <alpine dot LSU dot 2 dot 11 dot 1408200948520 dot 20733 at zhemvz dot fhfr dot qr> <alpine dot DEB dot 2 dot 11 dot 1408201012300 dot 1778 at laptop-mg dot saclay dot inria dot fr> <alpine dot LSU dot 2 dot 11 dot 1408201047030 dot 20733 at zhemvz dot fhfr dot qr> <alpine dot DEB dot 2 dot 11 dot 1408201122020 dot 1778 at laptop-mg dot saclay dot inria dot fr>
On Wed, 20 Aug 2014, Marc Glisse wrote:
> On Wed, 20 Aug 2014, Richard Biener wrote:
>
> > On Wed, 20 Aug 2014, Marc Glisse wrote:
> >
> > > On Wed, 20 Aug 2014, Richard Biener wrote:
> > >
> > > > Committed.
> > > >
> > > > Also makes visible a desirable change I plan for if-exprs. They
> > > > should behave like outer ifs and allow us to write that series
> > > > of pattern as
> > > >
> > > > (for op in eq ne
> > > > /* Simplify X * C1 CMP 0 to X CMP 0 if C1 is not zero. */
> > > > (simplify
> > > > (op (mult @0 INTEGER_CST@1) integer_zerop@2)
> > > > /* In fold-const.c we have this and the following patterns
> > > > combined because there we can "compute" the operator
> > > > to use by using swap_tree_comparison. */
> > > > (if (TYPE_OVERFLOW_UNDEFINED (TREE_TYPE (@0)))
> > > > (if (tree_int_cst_sgn (@1) > 0)
> > > > (op @0 @2))
> > > > (if (tree_int_cst_sgn (@1) < 0 && op == EQ_EXPR)
> > > > (ne @0 @2))
> > > > (if (tree_int_cst_sgn (@1) < 0 && op == NE_EXPR)
> > > > (eq @0 @2)))))
> > > >
> > > > that is, inner ifs have two operands, one condition and one
> > > > "result" (which can be another if). And the simplify
> > > > now has one mandatory match operand and at least one
> > > > result operand (if which all but the last have to be an
> > > > 'if').
> > >
> > > Not related to how you do "if" and such, but this simplification doesn't
> > > make
> > > sense. swap_tree_comparison preserves eq and ne, you only care that @1 is
> > > non-zero. It is for comparisons like lt that the sign can change the
> > > operation.
> >
> > Oops, true (the fold_comparison code doesn't restrict itself to
> > eq and ne). So for ne and eq the sign of @1 doesn't matter.
> > So we can improve here and do
> >
> > /* Simplify X * C1 CMP 0 to X CMP 0 if C1 is not zero. */
> > (for op in lt le eq ne ge gt
> > (simplify
> > (op (mult @0 INTEGER_CST@1) integer_zerop@2)
> > /* In fold-const.c we have this and the following pattern
> > combined because there we can "compute" the operator
> > to use by using swap_tree_comparison. Here we manage
> > to use only two patterns by swapping the operands instead
> > of changing the comparison code. */
> > (if (TYPE_OVERFLOW_UNDEFINED (TREE_TYPE (@0))
> > && tree_int_cst_sgn (@1) > 0))
> > (op @0 @2))
> > (simplify
> > (op (mult @0 INTEGER_CST@1) integer_zerop@2)
> > (if (TYPE_OVERFLOW_UNDEFINED (TREE_TYPE (@0))
> > && tree_int_cst_sgn (@1) < 0))
> > (op @2 @0)))
> >
> > right?
>
> Yes, that looks fine.
>
> (fold-const.c checks TREE_OVERFLOW(@1), I don't know about that)
Not sure why it does that.
> (we are obviously losing the warning)
Yes. I have no idea how to preserve those - I'd have to invent
a similar API as fold() does to defer warnings.
Other than that adding some special syntax just for the overflow
warnings is of course possible.
Richard.