[Mesa-dev] [PATCH 20/78] i965/nir/vec4: Implement load_uniform intrinsic

[Jason Ekstrand]

On Thu, Jul 2, 2015 at 3:53 AM, Iago Toral <itoral at igalia.com> wrote:
> On Thu, 2015-07-02 at 09:33 +0200, Iago Toral wrote:
>> On Tue, 2015-06-30 at 11:53 -0700, Jason Ekstrand wrote:
>> > On Fri, Jun 26, 2015 at 1:06 AM, Eduardo Lima Mitev <elima at igalia.com> wrote:
>> > > From: Iago Toral Quiroga <itoral at igalia.com>
>> > >
>> > > For the indirect case we need to take the index delivered by
>> > > NIR and compute the parent uniform that we are accessing (the one
>> > > that we uploaded to a surface) and the constant offset into that
>> > > surface.
>> > >
>> > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=89580
>> > > ---
>> > >  src/mesa/drivers/dri/i965/brw_vec4_nir.cpp | 27 +++++++++++++++++++++++++--
>> > >  1 file changed, 25 insertions(+), 2 deletions(-)
>> > >
>> > > diff --git a/src/mesa/drivers/dri/i965/brw_vec4_nir.cpp b/src/mesa/drivers/dri/i965/brw_vec4_nir.cpp
>> > > index 3fa8ca8..9a0ae25 100644
>> > > --- a/src/mesa/drivers/dri/i965/brw_vec4_nir.cpp
>> > > +++ b/src/mesa/drivers/dri/i965/brw_vec4_nir.cpp
>> > > @@ -569,10 +569,33 @@ vec4_visitor::nir_emit_intrinsic(nir_intrinsic_instr *instr)
>> > >     }
>> > >
>> > >     case nir_intrinsic_load_uniform_indirect:
>> > > +      has_indirect = true;
>> > >        /* fallthrough */
>> > > -   case nir_intrinsic_load_uniform:
>> > > -      /* @TODO: Not yet implemented */
>> > > +   case nir_intrinsic_load_uniform: {
>> > > +      int index = instr->const_index[0];
>> > > +      int uniform = nir_uniform_offset[index];
>> >
>> > Again, I don't know that this indirection is really needed.  We should
>> > make nir_lower_io just do the right thing for you here.
>>
>> Ok, I'll see what kind of changes we need to achieve that.
>>
>
> Initially, this was a consequence of the fact that we had not patched
> nir_lower_io to operate in units of vec4 for non-scalar shaders, so we
> needed a way to compute the vec4 offset from the scalar offset. With my
> patch to nir_lower_io that is not needed any more except for the fact
> that brw_nir.c calls nir_assign_var_locations_direct_first instead of
> nir_assign_var_locations for uniforms, which breaks the assumption that
> uniforms get locations in the same order as we find them in the shader.
> Because of that we still need a way to map what nir_lower_io delivers to
> a uniform number. Obviously, that function is very much tied to how the
> FS works (which splits uniforms in direct only and indirect) so I
> suppose it makes sense to call one or the other depending on whether
> this is a scalar shader or not.
>
> In summary: calling nir_assign_var_locations for uniforms in vec4
> shaders instead of nir_assign_var_locations_direct_first allows us to
> get rid of that nir_uniform_offset array mapping. Does this change look
> good to you?

That's fine to me.  I expected them to be different on different
backends anyway.

> Iago
>
>>
>> > > +      dest = get_nir_dest(instr->dest);
>> > > +
>> > > +      if (has_indirect) {
>> > > +         /* Split addressing into uniform and offset */
>> > > +         int offset = index - nir_uniform_driver_location[uniform];
>> > > +         assert(offset >= 0);
>> > > +
>> > > +         uniform -= offset;
>> > > +         assert(uniform >= 0);
>> > > +
>> > > +         src = src_reg(dst_reg(UNIFORM, uniform));
>> > > +         src.reg_offset = offset;
>> > > +         src_reg tmp = get_nir_src(instr->src[0], BRW_REGISTER_TYPE_D);
>> > > +         src.reladdr = new(mem_ctx) src_reg(tmp);
>> > > +      } else {
>> > > +         src = src_reg(dst_reg(UNIFORM, uniform));
>> > > +      }
>> > > +
>> > > +      emit(MOV(dest, src));
>> > >        break;
>> > > +   }
>> > >
>> > >     case nir_intrinsic_atomic_counter_read:
>> > >     case nir_intrinsic_atomic_counter_inc:
>> > > --
>> > > 2.1.4
>> > >
>> > > _______________________________________________
>> > > mesa-dev mailing list
>> > > mesa-dev at lists.freedesktop.org
>> > > http://lists.freedesktop.org/mailman/listinfo/mesa-dev
>> > _______________________________________________
>> > mesa-dev mailing list
>> > mesa-dev at lists.freedesktop.org
>> > http://lists.freedesktop.org/mailman/listinfo/mesa-dev
>>
>>
>> _______________________________________________
>> mesa-dev mailing list
>> mesa-dev at lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
>