On Fri, 21 Feb 2014, Bin.Cheng wrote:
> 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)?
PHI node doesn't matter but doesn't trigger stmt_ends_bb_p anyway.
It's better to do before the replacement because a stmt that may
have been stmt_ends_bb_p before the replacement might not be
after it (and thus we'd miss a cfgcleanup opportunity to merge
two blocks).
Richard.
> 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. */
>
>
>
>
--
Richard Biener <rguent...@suse.de>
SUSE / SUSE Labs
SUSE LINUX Products GmbH - Nuernberg - AG Nuernberg - HRB 16746
GF: Jeff Hawn, Jennifer Guild, Felix Imend"orffer
