This is the mail archive of the
mailing list for the GCC project.
Re: [PATCH]: Fix PR tree-optimization/27373
- From: "Richard Guenther" <richard dot guenther at gmail dot com>
- To: "Daniel Berlin" <dberlin at dberlin dot org>
- Cc: "GCC Patches" <gcc-patches at gcc dot gnu dot org>, dnovillo at redhat dot com
- Date: Tue, 16 May 2006 10:11:39 +0200
- Subject: Re: [PATCH]: Fix PR tree-optimization/27373
- References: <446909D1.email@example.com>
On 5/16/06, Daniel Berlin <firstname.lastname@example.org> wrote:
This fixes tree-opt/27373, an ICE in add_virtual_operand.
The actual cause of this bug is that forwprop is not correctly noticing
it needs to update the SMT info.
I expected forward_propagate_addr_expr_1 to return true if it changed
something, as that was what i thought it meant when it says "return true
if propagation is successful".
This is not the case, and it if it only replaces the *LHS*, which is
what happens here.
Because it didn't return true, we then think nothing has changed, and
don't touch the SMT info. In fact, something *did* change. It
propagates into the LHS of the expression, and changed the SMT info as a
However, if we change the one case it returned false before (around
/* Now see if the LHS node is an INDIRECT_REF using NAME. If so,
propagate the ADDR_EXPR into the use of NAME and fold the result. */
if (TREE_CODE (lhs) == INDIRECT_REF && TREE_OPERAND (lhs, 0) == name)
/* This should always succeed in creating gimple, so there is
no need to save enough state to undo this propagation. */
TREE_OPERAND (lhs, 0) = unshare_expr (TREE_OPERAND (stmt, 1));
so that it returns true, we then end up destroying definitions we
shouldn't (it looks like it thinks it can remove the statement, which
must be true of the other cases where we return true).
Yeah, the return value is true only if it did propagate into _all_ uses, so
concerning the stmt propagated to, the def is dead if true is returned. There
are some corner cases where we can propagate into only the rhs or lhs but
still have uses in the other one left - this is what you are hitting.
Rather than mess around more with the semantics it currently expects,
and end up rewriting half the pass, I just added a parameter to
forward_propagate_addr_expr_1 that says whether anything in use_stmt has
been changed, which is what we want to know for SMT usage update purposes.
The patch looks good, though I cannot approve it, obviously.