[Mesa-dev] [PATCH 3/5] i965/fs: Remove the requirement of no dead code for interference checks.

[Eric Anholt]

On Wed, 09 May 2012 00:43:47 -0700, Kenneth Graunke <kenneth at whitecape.org> wrote:
> On 05/08/2012 05:59 PM, Eric Anholt wrote:
> > This will be convenient when I want to comment out optimization code
> > to see the raw program being optimized, but more importantly will let
> > the interference check be used during optimization.
> > ---
> >   .../drivers/dri/i965/brw_fs_live_variables.cpp     |   24 ++++++++++----------
> >   1 file changed, 12 insertions(+), 12 deletions(-)
> >
> > diff --git a/src/mesa/drivers/dri/i965/brw_fs_live_variables.cpp b/src/mesa/drivers/dri/i965/brw_fs_live_variables.cpp
> > index c7ee582..46408da 100644
> > --- a/src/mesa/drivers/dri/i965/brw_fs_live_variables.cpp
> > +++ b/src/mesa/drivers/dri/i965/brw_fs_live_variables.cpp
> > @@ -226,19 +226,19 @@ fs_visitor::calculate_live_intervals()
> >   bool
> >   fs_visitor::virtual_grf_interferes(int a, int b)
> >   {
> > -   int start = MAX2(this->virtual_grf_def[a], this->virtual_grf_def[b]);
> > -   int end = MIN2(this->virtual_grf_use[a], this->virtual_grf_use[b]);
> > -
> > -   /* We can't handle dead register writes here, without iterating
> > -    * over the whole instruction stream to find every single dead
> > -    * write to that register to compare to the live interval of the
> > -    * other register.  Just assert that dead_code_eliminate() has been
> > -    * called.
> > +   int a_def = this->virtual_grf_def[a], a_use = this->virtual_grf_use[a];
> > +   int b_def = this->virtual_grf_def[b], b_use = this->virtual_grf_use[b];
> > +
> > +   /* If there's dead code (def but not use), it would break our test
> > +    * unless we consider it used.
> >       */
> 
> Sorry...I'm not quite sure what test you mean here.  The start < end 
> test at the end?  Yes, the dead code case would yield (1 << 30) < -1 and 
> fail that test, so we'd return false...aka "it doesn't 
> interfere"...which is the opposite of what you've special-cased here.
> 
> So if it does need to be marked as interfering, your code is correct.
> 
> What are the ramifications of saying that it does or doesn't interfere? 
>   (If you'd prefer to just point me at a paper, that's OK too...it's 
> been a while since I've really looked at register allocation so I'm 
> probably being dense.)
> 
> Regardless, I'm glad to see that it's now possible to comment out the 
> dead code pass.

Yeah, the assertion was to make sure we didn't hit the return false
case, and I decided the "right" thing to do was return true.

The requirement is that when you have a sequence of instructions (top to
bottom, just in columns for visual effect):

write a
             write b
read a
             (b is never read)

that b, though dead, is considered to be interfering with a.  Otherwise
you'll allocate it into the same spot as a and trash it.  Now, this
fails to distinguish from, say, the case:

             write b
write a
read a
             (b is never read)

where they don't actually interfere, but we do it this way because the
interval information we have wouldn't be different between that case
and:

             write b
write a
             write b
read a
             (b is never read)

But now that I think about it, we probably have a bug with:

             write b
             read b
write a
             write b
read a

where they aren't considered as interfering.  That case would have been
dealt with by the dead code pass (by removing the second write to b).
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 197 bytes
Desc: not available
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20120509/c68ec1c5/attachment.pgp>