<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Mon, Sep 22, 2014 at 10:49 AM, Matt Turner <span dir="ltr"><<a href="mailto:mattst88@gmail.com" target="_blank">mattst88@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="HOEnZb"><div class="h5">On Fri, Sep 19, 2014 at 8:14 PM, Jason Ekstrand <<a href="mailto:jason@jlekstrand.net">jason@jlekstrand.net</a>> wrote:<br>
><br>
><br>
> On Fri, Sep 19, 2014 at 5:16 PM, Matt Turner <<a href="mailto:mattst88@gmail.com">mattst88@gmail.com</a>> wrote:<br>
>><br>
>> On Fri, Sep 19, 2014 at 1:10 PM, Jason Ekstrand <<a href="mailto:jason@jlekstrand.net">jason@jlekstrand.net</a>><br>
>> wrote:<br>
>> > Signed-off-by: Jason Ekstrand <<a href="mailto:jason.ekstrand@intel.com">jason.ekstrand@intel.com</a>><br>
>> > ---<br>
>> >  src/mesa/drivers/dri/i965/brw_fs_dead_code_eliminate.cpp | 10<br>
>> > +++++-----<br>
>> >  1 file changed, 5 insertions(+), 5 deletions(-)<br>
>> ><br>
>> > diff --git a/src/mesa/drivers/dri/i965/brw_fs_dead_code_eliminate.cpp<br>
>> > b/src/mesa/drivers/dri/i965/brw_fs_dead_code_eliminate.cpp<br>
>> > index 697b44a..036875f 100644<br>
>> > --- a/src/mesa/drivers/dri/i965/brw_fs_dead_code_eliminate.cpp<br>
>> > +++ b/src/mesa/drivers/dri/i965/brw_fs_dead_code_eliminate.cpp<br>
>> > @@ -58,7 +58,7 @@ fs_visitor::dead_code_eliminate()<br>
>> >                 int var = live_intervals->var_from_reg(&inst->dst);<br>
>> >                 result_live = BITSET_TEST(live, var);<br>
>> >              } else {<br>
>> > -               int var = live_intervals->var_from_vgrf[inst->dst.reg];<br>
>> > +               int var = live_intervals->var_from_reg(&inst->dst);<br>
>> >                 for (int i = 0; i < inst->regs_written; i++) {<br>
>> >                    result_live = result_live || BITSET_TEST(live, var +<br>
>> > i);<br>
>><br>
>> This is wrong, isn't it? Before we get the base var and iterate 0<br>
>> through regs_written. After we're getting the var of the<br>
>> register+offset and then iterating.<br>
><br>
><br>
> No, in fact this hunk is what prompted me to make the change.  If we write<br>
> to vgrf3+2.0, then the previous version would tacitly assume that the offset<br>
> is 0 and treat it as if we were writing to vgrf3+0.0.<br>
<br>
</div></div>Not exactly. It just ORs the liveness for each register offset and<br>
uses that to determine whether it can remove the instruction writing<br>
to one offset.<br></blockquote><div><br></div><div>That would be the case if it went from 0 to virtual_grf_sizes[inst->dst.reg].  However, right now, it ORs the first regs_written regardless of where the destination starts.<br></div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
Changing that to remove the instruction based on the liveness of just<br>
the offset it's writing is probably fine, but that's a functional<br>
change, and this patch appears to be making non-functional changes.<br>
I'd want to split those into separate commits.<br>
</blockquote></div><br></div><div class="gmail_extra">I can do that. I don't care whether it's 1 or 2.<br></div><div class="gmail_extra">--Jason<br></div></div>