[Mesa-dev] [PATCH 07/12] i965/fs: Use the var_from_vgrf helper function instead of doing it manually

[Matt Turner]

On Fri, Sep 19, 2014 at 8:14 PM, Jason Ekstrand <jason at jlekstrand.net> wrote:
>
>
> On Fri, Sep 19, 2014 at 5:16 PM, Matt Turner <mattst88 at gmail.com> wrote:
>>
>> On Fri, Sep 19, 2014 at 1:10 PM, Jason Ekstrand <jason at jlekstrand.net>
>> wrote:
>> > Signed-off-by: Jason Ekstrand <jason.ekstrand at intel.com>
>> > ---
>> >  src/mesa/drivers/dri/i965/brw_fs_dead_code_eliminate.cpp | 10
>> > +++++-----
>> >  1 file changed, 5 insertions(+), 5 deletions(-)
>> >
>> > diff --git a/src/mesa/drivers/dri/i965/brw_fs_dead_code_eliminate.cpp
>> > b/src/mesa/drivers/dri/i965/brw_fs_dead_code_eliminate.cpp
>> > index 697b44a..036875f 100644
>> > --- a/src/mesa/drivers/dri/i965/brw_fs_dead_code_eliminate.cpp
>> > +++ b/src/mesa/drivers/dri/i965/brw_fs_dead_code_eliminate.cpp
>> > @@ -58,7 +58,7 @@ fs_visitor::dead_code_eliminate()
>> >                 int var = live_intervals->var_from_reg(&inst->dst);
>> >                 result_live = BITSET_TEST(live, var);
>> >              } else {
>> > -               int var = live_intervals->var_from_vgrf[inst->dst.reg];
>> > +               int var = live_intervals->var_from_reg(&inst->dst);
>> >                 for (int i = 0; i < inst->regs_written; i++) {
>> >                    result_live = result_live || BITSET_TEST(live, var +
>> > i);
>>
>> This is wrong, isn't it? Before we get the base var and iterate 0
>> through regs_written. After we're getting the var of the
>> register+offset and then iterating.
>
>
> No, in fact this hunk is what prompted me to make the change.  If we write
> to vgrf3+2.0, then the previous version would tacitly assume that the offset
> is 0 and treat it as if we were writing to vgrf3+0.0.

Not exactly. It just ORs the liveness for each register offset and
uses that to determine whether it can remove the instruction writing
to one offset.

Changing that to remove the instruction based on the liveness of just
the offset it's writing is probably fine, but that's a functional
change, and this patch appears to be making non-functional changes.
I'd want to split those into separate commits.