On Wed, Jun 13, 2018 at 10:51:41AM +0200, Richard Biener wrote:
> Your concern is for compile-time, not for correctness, right?
Yes.
> I think that
>
> /* If ARG0 and ARG1 are the same SAVE_EXPR, they are necessarily equal.
> We don't care about side effects in that case because the SAVE_EXPR
> takes care of that for us. In all other cases, two expressions are
> equal if they have no side effects. If we have two identical
> expressions with side effects that should be treated the same due
> to the only side effects being identical SAVE_EXPR's, that will
> be detected in the recursive calls below.
> If we are taking an invariant address of two identical objects
> they are necessarily equal as well. */
> if (arg0 == arg1 && ! (flags & OEP_ONLY_CONST)
> && (TREE_CODE (arg0) == SAVE_EXPR
> || (flags & OEP_MATCH_SIDE_EFFECTS)
> || (! TREE_SIDE_EFFECTS (arg0) && ! TREE_SIDE_EFFECTS (arg1))))
> return 1;
>
> is supposed to handle == SAVE_EXPRs and thus the hunk should be
> conditionalized on arg0 != arg1 or rather for OEP_LEXICOGRAPHIC
> always return true for arg0 == arg1?
The above handles the arg0 == arg1 case, sure, but that is not what
is compared in the -Wduplicated-branches testcases I've posted.
There we have different SAVE_EXPRs (one set for the then branch, another
for the else branch) which hash the same and we call operand_equal_p on it
then, but without some sort of caching (both in the inchash::add_expr
SAVE_EXPR case and operand_equal_p OEP_LEXICOGRAPHIC arg0 != arg1 SAVE_EXPR
case) we will check the same thing again and again.
Jakub
