This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH] Add un-distribution capabilities to tree reassoc (3rd try)
On Mon, 18 Aug 2008, Richard Guenther wrote:
> On Fri, 15 Aug 2008, Xinliang David Li wrote:
>
> > Richard Guenther wrote:
> > > On Thu, 14 Aug 2008, Xinliang David Li wrote:
> > >
> > > > > > > lhs = gimple_assign_lhs (stmt);
> > > > > > > rhs1 = gimple_assign_rhs1 (stmt);
> > > > > > > --- 1760,1775 ----
> > > > > > > /* If this was part of an already processed statement,
> > > > > > > we don't need to touch it again. */
> > > > > > > if (gimple_visited_p (stmt))
> > > > > > > ! {
> > > > > > > ! /* This statement might have become dead because of
> > > > > > > previous
> > > > > > > ! reassociations. */
> > > > > > > ! if (has_zero_uses (gimple_get_lhs (stmt)))
> > > > > > > ! {
> > > > > > > ! gsi_remove (&gsi, true);
> > > > > > > ! release_defs (stmt);
> > > > There seem to be a minor problem -- there needs to be an gsi_end_p check
> > > > and
> > > > break after the removal.
> > >
> > > Just by visual inspection or do you have a testcase? As we are walking
> > > from the end of the basic-block to the beginning we shouldn't hit the
> > > last statement again (note this is guarded by gimple_visited_p).
> > >
> > > Richard.
> >
> > A statement in dependence chain (lhs subtrees) of the linearized tree can
> > become dead due to the OPS list optimizations (including the factorization
> > work you did) -- if such a statement is in the first stmt in the bb after the
> > phis, gsi_remove will cause the iterator to point to the end marker.
> >
> > The problem is only triggered by some of my independent changes which I have
> > not submitted.
>
> I am checking the following (well, hard to do without a testcase, so
> you might want to double-check).
Bootstrapped/tested on x86_64-unknown-linux-gnu, applied to the trunk.
Richard.
> 2008-08-18 Richard Guenther <rguenther@suse.de>
>
> * tree-ssa-reassoc.c (reassociate_bb): Properly reset the
> statement iterator after statement removal.
>
> Index: tree-ssa-reassoc.c
> ===================================================================
> *** tree-ssa-reassoc.c (revision 139186)
> --- tree-ssa-reassoc.c (working copy)
> *************** reassociate_bb (basic_block bb)
> *** 1771,1776 ****
> --- 1771,1788 ----
> {
> gsi_remove (&gsi, true);
> release_defs (stmt);
> + /* We might end up removing the last stmt above which
> + places the iterator to the end of the sequence.
> + Reset it to the last stmt in this case which might
> + be the end of the sequence as well if we removed
> + the last statement of the sequence. In which case
> + we need to bail out. */
> + if (gsi_end_p (gsi))
> + {
> + gsi = gsi_last_bb (bb);
> + if (gsi_end_p (gsi))
> + break;
> + }
> }
> continue;
> }
>