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

[Jason Ekstrand]

On Mon, Sep 22, 2014 at 10:49 AM, Matt Turner <mattst88 at gmail.com> wrote:

> 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.
>

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.


> 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.
>

I can do that. I don't care whether it's 1 or 2.
--Jason
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20140922/b152920f/attachment.html>