On Thu, Feb 20, 2014 at 10:51 PM, Richard Biener <rguent...@suse.de> wrote:
>
> The following fixes an ICE I got when building libjava with a local
> patch. This causes us to substitute &MEM[&a, 5] into MEM[_3, 0]
> to MEM[&MEM[&a, 5], 0] and then asking stmt_ends_bb_p which doesn't
> expect such bogus MEM_REFs. The MEM_REF is canonicalized by
> calling fold_stmt on it later, but the fix is of course to move
> the marking of altered BBs before doing the actual substitution
> (only then we are sure to catch all previous bb-ending stmts).
>
> I also noticed we don't verify MEM_REFs on LHSs.
>
> Bootstrapped and tested on x86_64-unknown-linux-gnu, applied
> to trunk and branch (it's a regression uncovered by the fix for PR60115).
>
> Richard.
>
> 2014-02-20 Richard Biener <rguent...@suse.de>
>
> * tree-cfg.c (replace_uses_by): Mark altered BBs before
> doing the substitution.
> (verify_gimple_assign_single): Also verify bare MEM_REFs
> on the lhs.
>
> Index: gcc/tree-cfg.c
> ===================================================================
> --- gcc/tree-cfg.c (revision 207936)
> +++ gcc/tree-cfg.c (working copy)
> @@ -1677,6 +1677,11 @@ replace_uses_by (tree name, tree val)
>
> FOR_EACH_IMM_USE_STMT (stmt, imm_iter, name)
> {
> + /* Mark the block if we change the last stmt in it. */
> + if (cfgcleanup_altered_bbs
> + && stmt_ends_bb_p (stmt))
> + bitmap_set_bit (cfgcleanup_altered_bbs, gimple_bb (stmt)->index);
> +
> FOR_EACH_IMM_USE_ON_STMT (use, imm_iter)
> {
> replace_exp (use, val);
> @@ -1701,11 +1706,6 @@ replace_uses_by (tree name, tree val)
> gimple orig_stmt = stmt;
> size_t i;
>
> - /* Mark the block if we changed the last stmt in it. */
> - if (cfgcleanup_altered_bbs
> - && stmt_ends_bb_p (stmt))
> - bitmap_set_bit (cfgcleanup_altered_bbs, gimple_bb (stmt)->index);
> -
Hi Richard,
I also noticed this with local patch, but is it OK just to move above
code after fold_stmt? In other words, does phi node matter (according
to comments before cfgcleanup_altered_bbs)?
Thanks in advance.
> /* FIXME. It shouldn't be required to keep TREE_CONSTANT
> on ADDR_EXPRs up-to-date on GIMPLE. Propagation will
> only change sth from non-invariant to invariant, and only
> @@ -3986,7 +3986,9 @@ verify_gimple_assign_single (gimple stmt
> return true;
> }
>
> - if (handled_component_p (lhs))
> + if (handled_component_p (lhs)
> + || TREE_CODE (lhs) == MEM_REF
> + || TREE_CODE (lhs) == TARGET_MEM_REF)
> res |= verify_types_in_gimple_reference (lhs, true);
>
> /* Special codes we cannot handle via their class. */
--
Best Regards.
