On Mon, Sep 22, 2014 at 10:49 AM, Matt Turner <matts...@gmail.com> wrote:
> On Fri, Sep 19, 2014 at 8:14 PM, Jason Ekstrand <ja...@jlekstrand.net>
> wrote:
> >
> >
> > On Fri, Sep 19, 2014 at 5:16 PM, Matt Turner <matts...@gmail.com> wrote:
> >>
> >> On Fri, Sep 19, 2014 at 1:10 PM, Jason Ekstrand <ja...@jlekstrand.net>
> >> wrote:
> >> > Signed-off-by: Jason Ekstrand <jason.ekstr...@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
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
