Re: Patch to fix unintentional(?) pessimisation in dbr_schedule

[Richard Sandiford <rsandifo at redhat dot com>]

Richard Henderson <rth@redhat.com> writes:
> On Mon, Apr 05, 2004 at 11:52:17PM +0100, Richard Sandiford wrote:
>>   (a) dbr_schedule starts off with S, the registers live at D.  It will
>>       correctly detect that S includes "x" because it scans from the
>>       previous barrier and notices the assignment in B.
>
> Look at one of the other three alternatives you showed: ABCD.
> The set of registers live at D will be assumed to be the set of
> registers live at the barrier before C.  The scan forward from
> there will not see a set of x, so it'll never be considered live.

Hang on, you're confusing me. ;)  Let's requote the original code:

	if (always_true ())
	  x = foo();
	else
	  eventually_abort_but_not_marked_noreturn ();
	blah(x);

We're assuming here that gcc doesn't know that e_a_b_n_m_n() will always
abort, right?  So it thinks there's an execution path A->C->D.  In that
case, x's _pseudo_ register will be live on this path, meaning it must
be live before the call to e_a_b_n_m_n() and before the always_true()
conditional.  It can't magically spring to life on C->D.

So I don't see why there's a problem.  x's pseudo register is live
across a call.  Either it gets allocated a call-clobbered register
or it gets spilled on the stack.  In the latter case, there must be
code to restore it after executing C.

Even supposing there is a bug in the way the scan-from-barrier handles CALLs,
I don't see why that would imply the IOR is correct.  Remember that the code
I'm patching is only executed if the next jump after the insn is unconditional.
In other cases we rely entirely on the scan-from-previous-barrier technique
to calculate the live register set.

For example, let's extend the code as follows:

       	if (always_true ())
          {
	    x = foo();
            if (a)
              x = something_else ();
          }
	else
	  eventually_abort_but_not_marked_noreturn ();
	blah (x);
        if (b) return;

and assume it gets laid out as:

        1: if !always_true () goto 6
        2: x = foo ()
        3: if a goto 8
        4: x = something_else ()
        5: goto 8
        6: barrier
        7: eventually_abort_but_not_marked_noreturn ()
        8: blah (x)
        9: if b ...

When filling the delay slot in (3), we want to know what's live
at (8), i.e. D.  Now the first jump after (8) is (9), which is
conditional, so in this case we're relying entirely on the scan
from the previous barrier (6).  The code I patched isn't executed
at all.

Richard