Re: [tree-ssa]: PRE updates and note on bug

[Daniel Berlin <dberlin at dberlin dot org>]

On Tuesday, July 22, 2003, at 1:21 PM, Andrew MacLeod wrote:

> On Tue, 2003-07-22 at 12:57, Daniel Berlin wrote:
> > > I thought we resolved this last week? You had a patch for
> > > remove_stmt()...
> > >
> > > Or did it not fix your problem?
> >
> > If you apply the following patch, you can see where we still have
> > problems (this includes when a cond_expr is the end of the bb, and we
> > remove it, when we remove empty statements and the previous statement 
> > is
> > an empty statement, etc).
> >
> > We probably should use this patch (without the #if 0'd code, of 
> > course, i
> > was just trying to start attacking the various cases) once all the 
> > cases
> > are handled, or to force ourselves to fix the bugs.
>
> but....  are you sure you are looking at the right thing?
It could be the reverse iterators, of course.

> Im not convinced that it isn't something else.. like a bug in the
> reverse iterators or a missing bb_for_stmt...
Where, exactly?

> out_of_ssa doesnt use the reverse iterators, but it does use the
> FOR_EACH_STMT_IN_REVERSE macro because its processes things backwards,
> and its not having any problems. If things were screwed up by DCE,
> something would get the wrong live range, or get missed, and I would
> expect something to break later on.

As I said, it could be the reverse iterators.
If the end of the bb is an empty statement, bsi = bsi_last (bb); 
bsi_prev (bsi), doesn't work.
If it should, then the reverse iterators are broken.
If the end of the bb shouldn't be an empty statement, then the bb ends 
aren't being updated.

> If the end of block pointer is actually set wrong, the forward 
> iterators
> would be missing stmts or something I would think.
Nope. They skip the empty statements, so they don't see the problem.

> If a stmt has been inserted into a block and bb_for_stmt wasn't set
> properly, it would  cause a lot of these problems too. For instance:
> is 33 the right number of stmts in the block? If a stmt has been added
> (say the 34th of 35), and its block number wasn't set, the forward
> iterators would quit at number 33. The reverse iterator would start 
> with
> #35, step back one, find a stmt without the correct block and quit.
> Giving you 1...
>
> Note that the end block pointer doesn't have to be a non-null stmt, it
> can point to anything it wants. It doesnt have to be a BSI valid stmt.
> Neither does the head pointer. All that matters is that bb_for_stmt is
> set right on them all.  My guess is since you are only encountering 
> this
> in PRE

This is a false statement, on two counts.
1. The patch I pasted inserts the check when *DCE* runs, which is the 
first thing that runs.
Not when PRE would run.
It has nothing to do with PRE at all.
You will see the problem any time you use reverse iterators.
which is #2
2. Nothing else uses reverse iterators.


Please apply the patch, which doesn't have anything to do with PRE (it 
only runs during DCE, in fact), and you'll see it is screwed up long 
before PRE even could run.

>  that somewhere you've messed up the BB value.

No, I haven't.
The check occurs *before* PRE runs.
Not after or during PRE.
I haven't done anything.


> try walking the trees from bb->head_tree_p to bb->end_tree_p and check
> bb_for_stmt on each element.
>
> Andrew