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 <el...@igalia.com>
> > wrote:
> > > From: Iago Toral Quiroga <ito...@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?
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@lists.freedesktop.org
> > > http://lists.freedesktop.org/mailman/listinfo/mesa-dev
> > _______________________________________________
> > mesa-dev mailing list
> > mesa-dev@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
