On Mon, Jan 09, 2017 at 11:57:48AM +0100, Richard Biener wrote:
> On Mon, 9 Jan 2017, Marek Polacek wrote:
>
> > On Thu, Jan 05, 2017 at 04:41:28PM +0100, Jakub Jelinek wrote:
> > > On Thu, Jan 05, 2017 at 04:39:40PM +0100, Marek Polacek wrote:
> > > > Coming back to this...
> > >
> > > > > Right, after h0 == h1 is missing && operand_equal_p (thenb, elseb, 0)
> > > > > or so (the exact last operand needs to be figured out).
> > > > > OEP_ONLY_CONST is certainly wrong, we want the same VAR_DECLs to mean
> > > > > the
> > > > > same thing. 0 is a tiny bit better, but still it will give up on
> > > > > e.g. pure
> > > > > and other calls. OEP_PURE_SAME is tiny bit better than that, but
> > > > > still
> > > > > calls with the same arguments to the same function will not be
> > > > > considered
> > > > > equal, plus likely operand_equal_p doesn't handle STATEMENT_LIST etc.
> > > > > So maybe we need another OEP_* mode for this.
> > > >
> > > > Yea, if I add "&& operand_equal_p (thenb, elseb, 0)" then this warning
> > > > doesn't
> > > > trigger for certain cases, such as MODIFY_EXPR, RETURN_EXPR, probably
> > > > STATEMENT_LIST and others. So I suppose I could introduce a new OEP_
> > > > mode for
> > > > this (names? OEP_EXTENDED?) and then in operand_equal_p in case
> > > > tcc_expression
> > > > do
> > > >
> > > > case MODIFY_EXPR:
> > > > if (flags & OEP_EXTENDED)
> > > > // compare LHS and RHS of both
> > > >
> > > > ?
> > >
> > > Yeah. Not sure what is the best name for that. Maybe Richi has some
> > > clever
> > > ideas.
> >
> > Here it is. The changes in operand_equal_p should only trigger with the new
> > OEP_LEXICOGRAPHIC, and given the macro location issue, the warning isn't yet
> > enabled by neither -Wall nor -Wextra, so this all should be safe.
> >
> > Bootstrapped/regtested on x86_64-linux, ok for trunk?
>
> @@ -2722,6 +2722,9 @@ combine_comparisons (location_t loc,
> If OEP_ADDRESS_OF is set, we are actually comparing addresses of
> objects,
> not values of expressions.
>
> + If OEP_LEXICOGRAPHIC is set, then also handle expressions such as
> + MODIFY_EXPR, RETURN_EXPR, as well as STATEMENT_LISTs.
> +
>
> I'd say "also handle expressions with side-effects such as ..."
>
> otherwise the middle-end changes look good to me - I'll defer to
> C FE maintainers for the rest.
Thanks, I'll fix it up.
Marek
