On Tue, 8 Feb 2022, Michael Matz wrote:

> Hello,
> 
> On Tue, 8 Feb 2022, Richard Biener wrote:
> 
> > > int state = 2, *p, camefrom1 = 0;
> > > for (;;) switch (state) {
> > >   case 1: 
> > >   case 2: ;
> > >     int i;
> > >     if (state != 1) { p = &i; i = 0; }
> > >     if (state == 1) { (*p)++; return *p; }
> > >     state = 1;
> > >     continue;
> > > }
> > > 
> > > Note how i is initialized during state 2, and needs to be kept 
> > > initialized 
> > > during state 1, so there must not be a CLOBBER (birth or other) at the 
> > > point of the declaration of 'i'.  AFAICS in my simple tests a DECL_EXPR 
> > > for 'i' is placed with the statement associated with 'case 2' label, 
> > > putting a CLOBBER there would be the wrong thing.  If the decl had an 
> > > initializer it might be harmless, as it would be overwritten at that 
> > > place, but even so, in this case there is no initializer.  Hmm.
> > 
> > You get after gimplification:
> > 
> >   state = 2;
> >   camefrom1 = 0;
> >   <D.1989>:
> >   switch (state) <default: <D.1996>, case 1: <D.1983>, case 2: <D.1984>>
> >   {
> >     int i;
> > 
> >     try
> >       {
> >         i = {CLOBBER(birth)};  /// ignore, should go away
> >         <D.1983>:
> >         <D.1984>:
> >         i = {CLOBBER(birth)};
> 
> I think this clobber here would be the problem, because ...
> 
> > which I think is OK?  That is, when the abstract machine
> > arrives at 'int i;' then the previous content in 'i' goes
> > away?  Or would
> > 
> > int foo()
> > {
> >    goto ick;
> > use:
> >    int i, *p;
> >    return *p;
> > ick:
> >    i = 1;
> >    p = &i;
> >    goto use;
> > 
> > }
> > 
> > require us to return 1?
> 
> ... I think this is exactly the logical consequence of lifetime of 'i' 
> being the whole block.  We need to return 1. (Joseph: correct me on that! 
> :) )  That's what I was trying to get at with my switch example as well.
> 
> > With the current patch 'i' is clobbered before the return.
> > 
> > > Another complication arises from simple forward jumps:
> > > 
> > >   goto forw;
> > >   {
> > >     int i;
> > >     printf("not reachable\n");
> > >   forw:
> > >     i = 1;
> > >   }
> > > 
> > > Despite the jump skiping the unreachable head of the BLOCK, 'i' needs to 
> > > be considered birthed at the label.  (In a way the placement of births 
> > > exactly mirrors the placements of deaths (of course), every path from 
> > > outside a BLOCK to inside needs to birth-clobber all variables (in C), 
> > > like every path leaving needs to kill them.  It's just that we have a 
> > > convenient construct for the latter (try-finally), but not for the former)
> > 
> > The situation with an unreachable birth is handled conservatively
> > since we consider a variable without a (visible at RTL expansion time)
> > birth to conflict with all other variables.
> 
> That breaks down when a birth is there (because it was otherwise 
> reachable) but not on the taken path:
> 
>   if (nevertrue)
>     goto start;
>   goto forw;
>   start:
>   {
>     int i;
>     printf("not really reachable, but unknowingly so\n");
>   forw:
>     i = 1;
>   }

I think to cause breakage you need a use of 'i' on the side-entry
path that is not reachable from the path with the birth.  I guess sth like

   if (nevertrue)
     goto start;
   goto forw;
   start:
   {
     int i = 0;
     printf("not really reachable, but unknowingly so\n");
     goto common;
   forw:
     i = 1;
   common:
     foo (&i);
   }

if we'd have a variable that's live only on the side-entry path
then it could share the stack slot with 'i' this way, breaking
things (now we don't move CLOBBERs so it isn't easy to construct
such case).  The present dataflow would, for the above, indeed
compute 'i' not live in the forw: block.

> > I don't see a good way to have a birth magically appear at 'forw' 
> > without trying to argue that the following stmt is the first mentioning 
> > the variable.
> 
> That's what my 'Hmm' aluded to :)  The only correct and precise way I see 
> is to implement something similar like try-finally topside-down.  An 
> easier but less precise way is to place the births at the (start of) 
> innermost block containing the decl _and all jumps into the block_.  Even 
> less presice, but perhaps even easier is to place the births for decls in 
> blocks with side-entries into the function scope (and for blocks without 
> side entries at their start).
> 
> Except for switches side-entries are really really seldom, so we might 
> usefully get away with that latter solution.  And for switches it might be 
> okay to put the births at the block containing the switch (if it itself 
> doesn't have side entries, and the switch block doesn't have side 
> entries except the case labels).
> 
> If the birth is moved to outward blocks it might be best if also the 
> dealloc/death clobbers are moved to it, otherwise there might be paths 
> containing a birth but no death.
> 
> The less precise you get with those births the more non-sharing you'll 
> get, but that's the price.

Yes, sure.  I don't see a good way to place births during gimplification
then.  The end clobbers rely on our EH lowering machinery.  For
the entries we might be able to compute GIMPLE BIND transitions during
BIND lowering if we associate labels with BINDs.  There should be
a single fallthru into the BIND at this point.  We could put a flag
on the goto destination labels whether they are reached from
an outer BIND.

 goto inner;
 {
   {
    int i;
     {
       int j;
inner:
       goto middle;
     }
    middle:
   }
 }

Since an entry CLOBBER is also a clobber we have to insert birth
clobbers for all decls of all the binds inbetwee the goto source
and the destination.  So for the above case on the edge to
inner: have births for i and j and at the edge to middle we'd
have none.

Requires some kind of back-mapping from label to goto sources
that are possibly problematic.  One issue is that GIMPLE
lowering re-builds the BLOCK tree (for whatever reason...),
so I'm not sure if we need to do it before that (for correctness).

Does that make sense?

Richard.

Reply via email to