On Tue, 8 Feb 2022, Michael Matz wrote:
> Hello,
>
> On Tue, 8 Feb 2022, Richard Biener wrote:
>
> > int foo(int always_true_at_runtime)
> > {
> > {
> > int *p;
> > if (always_true_at_runtime)
> > goto after;
> > lab:
> > return *p;
> > after:
> > int i = 0;
> > p = &i;
> > if (always_true_at_runtime)
> > goto lab;
> > }
> > return 0;
> > }
> >
> > For the implementation I considered starting lifetime at a DECL_EXPR
> > if it exists and otherwise at the start of the BIND_EXPR. Note the
> > complication is that control flow has to reach the lifetime-start
> > clobber before the first access. But that might also save us here,
> > since for the example above 'i' would be live via the backedge
> > from goto lab.
> >
> > That said, the existing complication is that when the gimplifier
> > visits the BIND_EXPR it has no way to know whether there will be
> > a following DECL_EXPR or not (and the gimplifier notes there are
> > cases a DECL_EXPR variable is not in a BIND_EXPR). My plan is to
> > compute this during one of the body walks the gimplifier performs
> > before gimplifying.
> >
> > Another complication is that at least the C frontend + gimplifier
> > combination for
> >
> > switch (i)
> > {
> > case 1:
> > int i;
> > break;
> > }
> >
> > will end up having the BIND_EXPR mentioning 'i' containing the
> > case label, so just placing the birth at the BIND will make it
> > unreachable as
> >
> > i = BIRTH;
> > case_label_for_1:
> > ...
>
> I think anything that needs to happen (conceptually) during the jump from
> switch to case-label needs to happen right before the jump, that might
> mean creating a new fake BLOCK that contains just the jump and the
> associated births.
>
> > conveniently at least the C frontend has a DECL_EXPR for 'i'
> > which avoids this and I did not find a way (yet) in the gimplifier
> > to rectify this when gimplifying switches.
>
> In C a 'case' label is nothing else than a normal label, it doesn't start
> it's own block or the like. So also for that one the births would need to
> be at the start of their containing blocks.
>
> > So there's still work to do but I think that starting the lifetime at a
> > DECL_EXPR if it exists is the way to go?
>
> Depends on where the DECL_EXPR is exactly placed, but probably it wouldn't
> be okay. You can't place the birth at the fall-through path, because this
> needs to work (basically your jump example above rewritten as switch):
>
> 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)};
if (state != 1) goto <D.1991>; else goto <D.1992>;
<D.1991>:
p = &i;
i = 0;
<D.1992>:
if (state == 1) goto <D.1993>; else goto <D.1994>;
<D.1993>:
_1 = *p;
_2 = _1 + 1;
*p = _2;
D.1995 = *p;
// predicted unlikely by early return (on trees) predictor.
return D.1995;
<D.1994>:
state = 1;
// predicted unlikely by continue predictor.
goto <D.1987>;
}
finally
{
i = {CLOBBER(eol)};
}
}
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? 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.
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.
Richard.
