Re: [tree-ssa] COND_EXPR lowering

[Andrew MacLeod <amacleod at redhat dot com>]

On Wed, 2003-10-08 at 14:32, Zdenek Dvorak wrote:
> Hello,

> 
> GOTOs in branches are not statements.  The whole cond_expr is considered
> a single statement.  IMHO this is the whole point of the lowering -- to
> simplify structured statements.  I don't see any problems with this --
> everything that works over "paths" should work over cfg (and currently
> it also does).  Not that it would be hard to add the annotation bits you
> mention, but making this extra work unless it is actually needed by some
> pass seems useless to me.

I understand, I was just trying to think through if we might run into
problem. If the GOTO's aren't stmt's, then why not just put the 2 target
labels into the COND_EXPR, and ditch the GOTO_EXPR competely. This would
also resolve the problem with TDF_SLIM and printing things out...

If its for compatibility with GENERIC so that it still looks identical,
which I assume it is, then they are going to be treated as stmt's
sometimes... and not others.  Im just trying to get a consistant picture
:-). We might very well state that in GIMPLE they aren't stmt's, but are
always in this format, or something to that effect. I just want to make
sure we've fully considered the situation.

Furthermore, if this is the case, we ought to provide a mechanism to
create COND_EXPR's, so that no one forgets and makes one that isnt the
right format accidentally.  At least verify_flow_info now checks it,
presumably on a regular basis? when is that called? during things like
cfg_cleanup?


> > >   
> > > !   if (!bb->succ)
> > > !     return;
> > > !   if (bb->succ->succ_next)
> > > !     {
> > > !       val = COND_EXPR_COND (cond_expr);
> > > !       taken_edge = find_taken_edge (bb, val);
> > > !     }
> > > !   else
> > > !     taken_edge = bb->succ;
> > > ! 
> > 
> > When would !bb->succ not be an error.
> 
> When both successor blocks were removed.  This may happen in cases when
> the block becomes unreachable (I don't remember details just now, but
> it happened to me), and is later in the cleanup removed.

Just to follow through, oughtn't things that goto other things be
removed before the thing they goto? ha. parse that :-) So its possible
that targets of GOTO's are removed before the GOTO itself? I would hope
that its just an ordering thing.. ie we are removing a bunch of blocks,
of which the COND_EXPR is in one and the target is in another, and we
just happen to remove this one first.  If thats not the case, then Im a
little more nervous about it. 



> > find_taken_edge is called from switch processing as well, and a
> > SWITCH_EXPr is going to fail this test (it's a ctrl_structure). So I
> > don't think we ought to change this just yet.
> 
> is_ctrl_structure implies is_ctrl_stmt.

Doh, my bad.

> > >   }
> > 
> > I think I'd rather teach dump_generic_node that under TDF_SLIM, if
> > *both* sides of a COND_EXPR are GOTO's to also dump out the goto's. That
> > was the interface stays consistant.. ie, there are lots of other places
> > that we print stmt's and pass in TDF_SLIM... so I think it ought to show
> > the GOTO's.
> 
> I am not sure whether it would not break other dumps; I will try.

Yeah, Im not sure what it'll do to generic dumps... we might see the
goto twice... Not sure. we have to change it somewhere else to *not*
decend down into the arms of a COND_EXPR if both arms a goto.

> > 
> > These changes do not seem relevant to COND_EXPr lowering.. I assume its
> > from something else? So we oughta remove these changes from the patch.
> 
> They are here because cond_expr lowering exposed some new problems with
> scope assignment, so it is also necessary to call fixup_var_scope in
> out-of-ssa pass, when variables are no longer inside SSA_NAME_VAR.
> 
> > 
> > This looks like more unrelated scoping changes?
> 
> yes, it is just a problem exposed by the patch.  Since I could not
> reproduce it without the patch, I did not post it separately.
> 

What kinds of scoping problems?  out of ssa certainly shouldn't be
promoting anything out of its scope. In order for a variable to require
a copy on an edge, it must be valid in both the src and dest block since
the value is used in both. I'd like to see an instance of when scoping
is affected by COND_EXPr lowering. In theory it shouldn't be affected...

If a scoping problem that is caused from somewhere else and just shows
up here, I'd rather fix it in the originating location. Its certainly
possible something has been undiscovered up to now :-)


Andrew