[Mesa-dev] [PATCH] i965/nir: Mark const index UBO surfaces as used

[Iago Toral]

On Thu, 2015-10-29 at 10:44 -0700, Jordan Justen wrote:
> On 2015-10-29 02:17:20, Iago Toral wrote:
> > On Thu, 2015-10-29 at 00:50 -0700, Jordan Justen wrote:
> > > Signed-off-by: Jordan Justen <jordan.l.justen at intel.com>
> > > ---
> > >  src/mesa/drivers/dri/i965/brw_fs_nir.cpp   | 6 ++++--
> > >  src/mesa/drivers/dri/i965/brw_vec4_nir.cpp | 6 ++++--
> > >  2 files changed, 8 insertions(+), 4 deletions(-)
> > > 
> > > diff --git a/src/mesa/drivers/dri/i965/brw_fs_nir.cpp b/src/mesa/drivers/dri/i965/brw_fs_nir.cpp
> > > index 4950ba4..6d69e96 100644
> > > --- a/src/mesa/drivers/dri/i965/brw_fs_nir.cpp
> > > +++ b/src/mesa/drivers/dri/i965/brw_fs_nir.cpp
> > > @@ -1438,8 +1438,10 @@ fs_visitor::nir_emit_intrinsic(const fs_builder &bld, nir_intrinsic_instr *instr
> > >        fs_reg surf_index;
> > >  
> > >        if (const_index) {
> > > -         surf_index = fs_reg(stage_prog_data->binding_table.ubo_start +
> > > -                             const_index->u[0]);
> > > +         unsigned index = stage_prog_data->binding_table.ubo_start +
> > > +                          const_index->u[0];
> > > +         surf_index = fs_reg(index);
> > > +         brw_mark_surface_used(prog_data, index);
> > >        } else {
> > >           /* The block index is not a constant. Evaluate the index expression
> > >            * per-channel and add the base UBO index; we have to select a value
> > > diff --git a/src/mesa/drivers/dri/i965/brw_vec4_nir.cpp b/src/mesa/drivers/dri/i965/brw_vec4_nir.cpp
> > > index 0f04f65..efbdaa9 100644
> > > --- a/src/mesa/drivers/dri/i965/brw_vec4_nir.cpp
> > > +++ b/src/mesa/drivers/dri/i965/brw_vec4_nir.cpp
> > > @@ -749,8 +749,10 @@ vec4_visitor::nir_emit_intrinsic(nir_intrinsic_instr *instr)
> > >           /* The block index is a constant, so just emit the binding table entry
> > >            * as an immediate.
> > >            */
> > > -         surf_index = src_reg(prog_data->base.binding_table.ubo_start +
> > > -                              const_block_index->u[0]);
> > > +         unsigned index = prog_data->base.binding_table.ubo_start +
> > > +                          const_block_index->u[0];
> > > +         surf_index = src_reg(index);
> > > +         brw_mark_surface_used(&prog_data->base, index);
> > >        } else {
> > >           /* The block index is not a constant. Evaluate the index expression
> > >            * per-channel and add the base UBO index; we have to select a value
> > 
> > Is this necessary? As far as I can see, the generator opcodes will mark
> > surfaces for which we have a constant index as used, so this should not
> > be necessary.
> 
> Ah. You are correct.
> 
> I was debugging a UBO+SSBO test, and I noticed the discrepancy between
> SSBO and UBO, but it appears SSBO has to mark it in brw_*_nir.cpp,
> whereas the generator code can also mark it for the constant case for
> UBOs.
> 
> Maybe we should just handle this in brw_*_nir.cpp for consistency?

Yeah, I think so. I'll send a patch for this.

Iago