On Jun 30, 2015 1:05 AM, "Iago Toral" <ito...@igalia.com> wrote:
>
> Hi Jason,
>
> On Mon, 2015-06-29 at 16:22 -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>
> > >
> > > This is based on similar code existing in vec4_visitor. It builds the
> > > uniform register file iterating through each uniform variable. It
> > > also stores the index of each register at the corresponding offset
> > > in a map. This map will later be used by load_uniform intrinsic
> > > instructions to build the correct UNIFORM source register.
> > >
> > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=89580
> > > ---
> > > src/mesa/drivers/dri/i965/brw_vec4.h | 2 +
> > > src/mesa/drivers/dri/i965/brw_vec4_nir.cpp | 115
++++++++++++++++++++++++++++-
> > > 2 files changed, 114 insertions(+), 3 deletions(-)
> > >
> > > diff --git a/src/mesa/drivers/dri/i965/brw_vec4.h
b/src/mesa/drivers/dri/i965/brw_vec4.h
> > > index 673df4e..6535f19 100644
> > > --- a/src/mesa/drivers/dri/i965/brw_vec4.h
> > > +++ b/src/mesa/drivers/dri/i965/brw_vec4.h
> > > @@ -414,6 +414,8 @@ public:
> > > src_reg *nir_inputs;
> > > int *nir_outputs;
> > > brw_reg_type *nir_output_types;
> > > + unsigned *nir_uniform_offset;
> > > + unsigned *nir_uniform_driver_location;
> > >
> > > protected:
> > > void emit_vertex();
> > > diff --git a/src/mesa/drivers/dri/i965/brw_vec4_nir.cpp
b/src/mesa/drivers/dri/i965/brw_vec4_nir.cpp
> > > index 2d457a6..40ec66f 100644
> > > --- a/src/mesa/drivers/dri/i965/brw_vec4_nir.cpp
> > > +++ b/src/mesa/drivers/dri/i965/brw_vec4_nir.cpp
> > > @@ -106,19 +106,128 @@ vec4_visitor::nir_setup_outputs(nir_shader
*shader)
> > > void
> > > vec4_visitor::nir_setup_uniforms(nir_shader *shader)
> > > {
> > > - /* @TODO: Not yet implemented */
> > > + uniforms = 0;
> > > +
> > > + nir_uniform_offset =
> > > + rzalloc_array(mem_ctx, unsigned, this->uniform_array_size);
> > > + memset(nir_uniform_offset, 0, this->uniform_array_size *
sizeof(unsigned));
> >
> > rzalloc memsets the whole thing to 0 for you, this memset is redundant.
> >
> > > +
> > > + nir_uniform_driver_location =
> > > + rzalloc_array(mem_ctx, unsigned, this->uniform_array_size);
> > > + memset(nir_uniform_driver_location, 0,
> > > + this->uniform_array_size * sizeof(unsigned));
> >
> > Same here.
>
> Oh, right.
>
> > > +
> > > + if (shader_prog) {
> > > + foreach_list_typed(nir_variable, var, node, &shader->uniforms)
{
> > > + /* UBO's, atomics and samplers don't take up space in the
> > > + uniform file */
> > > + if (var->interface_type != NULL ||
var->type->contains_atomic() ||
> > > + type_size(var->type) == 0) {
> >
> > I'm curious as to why you have this extra type_size() == 0 condition.
> > We don't have that in the FS NIR code. What caused you to add it?
>
> Take this piglit test for example:
> bin/textureSize vs isampler1D -auto -fbo
>
> here, 'tex' is a uniform of size 0 since type_size() returns 0 for all
> sampler types. If we do not ignore these, we will try to store uniform
> information for them in the various structures we have to track uniform
> data, like uniform_size[] and others. The size allocated for these
> arrays is computed by in the vec4_visitor constructor based on
> stage_prog_data->nr_params (uniform_array_size) and that does not seem
> to make room for zero-sized uniforms. Without that check we would
> process more uniforms than uniform_array_size and overflow the arrays we
> allocate to track uniform information. I understand that
> stage_prog_data->nr_params does not track uniforms that don't use
> register space, so skipping uniforms with no size seems to make sense
> here.
>
> Notice that this is done in the current vec4_visitor too, when we visit
> the variable in vec4_visitor::visit(ir_variable *ir), for ir_var_uniform
> there is this code:
>
> if (ir->is_in_uniform_block() || type_size(ir->type) == 0)
> return;
For some reason, I was thinking textures were in their own list but now
that I look again, they are in the uniforms list. I guess this makes sense.
> > > + continue;
> > > + }
> > > +
> > > + assert(uniforms < uniform_array_size);
> > > + this->uniform_size[uniforms] = type_size(var->type);
> > > +
> > > + if (strncmp(var->name, "gl_", 3) == 0)
> > > + nir_setup_builtin_uniform(var);
> > > + else
> > > + nir_setup_uniform(var);
> > > + }
> > > + } else {
> > > + /* ARB_vertex_program is not supported yet */
> > > + assert("Not implemented");
> > > + }
> > > }
> > >
> > > void
> > > vec4_visitor::nir_setup_uniform(nir_variable *var)
> > > {
> > > - /* @TODO: Not yet implemented */
> > > + int namelen = strlen(var->name);
> > > +
> > > + /* The data for our (non-builtin) uniforms is stored in a series
of
> > > + * gl_uniform_driver_storage structs for each subcomponent that
> > > + * glGetUniformLocation() could name. We know it's been set up
in the same
> > > + * order we'd walk the type, so walk the list of storage and find
anything
> > > + * with our name, or the prefix of a component that starts with
our name.
> > > + */
> > > + unsigned offset = 0;
> > > + for (unsigned u = 0; u < shader_prog->NumUniformStorage; u++) {
> > > + struct gl_uniform_storage *storage =
&shader_prog->UniformStorage[u];
> > > +
> > > + if (storage->builtin)
> > > + continue;
> > > +
> > > + if (strncmp(var->name, storage->name, namelen) != 0 ||
> > > + (storage->name[namelen] != 0 &&
> > > + storage->name[namelen] != '.' &&
> > > + storage->name[namelen] != '[')) {
> > > + continue;
> > > + }
> > > +
> > > + gl_constant_value *components = storage->storage;
> > > + unsigned vector_count = (MAX2(storage->array_elements, 1) *
> > > + storage->type->matrix_columns);
> >
> > In the FS backend, we simply use storage->type->component_slots().
> > Why can't we do that here? It seems to be performing the same
> > calculation.
>
> This does not seem to do what we want in the vec4 backend. For example,
> consider the following piglit test:
>
> tests/spec/gl-3.1/attributeless-vertexid.shader_test
>
> In that test we have something like this:
>
> vec2[](
> vec2(-1, 1),
> vec2(-1,-1),
> vec2( 1,-1),
> vec2( 1, 1)
> );
>
> And for that I see:
>
> storage->type->component_slots() = 2
>
> This is because storage->type is glsl_type::_vec2_type. This is not
> convenient for us in the vec4 backend because it is giving a scalar
> count and we want to compute the count in units of vec4.
Sorry I missed the fact that you were only multiplying by matrix_columns
and not rows. This is probably fine.
--Jason
> > > +
> > > + for (unsigned s = 0; s < vector_count; s++) {
> > > + assert(uniforms < uniform_array_size);
> > > + uniform_vector_size[uniforms] =
storage->type->vector_elements;
> > > +
> > > + int i;
> > > + for (i = 0; i < uniform_vector_size[uniforms]; i++) {
> > > + stage_prog_data->param[uniforms * 4 + i] = components;
> > > + components++;
> > > + }
> > > + for (; i < 4; i++) {
> > > + static gl_constant_value zero = { 0.0 };
> >
> > This should probably be const.
>
> Yes, I will fix this.
>
> > > + stage_prog_data->param[uniforms * 4 + i] = &zero;
> > > + }
> > > +
> > > + int uniform_offset = var->data.driver_location + offset;
> > > + nir_uniform_offset[uniform_offset] = uniforms;
> > > + offset++;
> > > +
> > > + nir_uniform_driver_location[uniforms] =
var->data.driver_location;
> > > + uniforms++;
> > > + }
> > > + }
> > > }
> > >
> > > void
> > > vec4_visitor::nir_setup_builtin_uniform(nir_variable *var)
> > > {
> > > - /* @TODO: Not yet implemented */
> > > + const nir_state_slot *const slots = var->state_slots;
> > > + assert(var->state_slots != NULL);
> > > +
> > > + unsigned offset = 0;
> > > + for (unsigned int i = 0; i < var->num_state_slots; i++) {
> > > + /* This state reference has already been setup by ir_to_mesa,
> > > + * but we'll get the same index back here. We can reference
> > > + * ParameterValues directly, since unlike brw_fs.cpp, we never
> > > + * add new state references during compile.
> > > + */
> > > + int index = _mesa_add_state_reference(this->prog->Parameters,
> > > + (gl_state_index
*)slots[i].tokens);
> > > + gl_constant_value *values =
> > > + &this->prog->Parameters->ParameterValues[index][0];
> > > +
> > > + assert(this->uniforms < uniform_array_size);
> > > +
> > > + for (unsigned j = 0; j < 4; j++)
> > > + stage_prog_data->param[this->uniforms * 4 + j] =
> > > + &values[GET_SWZ(slots[i].swizzle, j)];
> > > +
> > > + this->uniform_vector_size[this->uniforms] =
> > > + (var->type->is_scalar() || var->type->is_vector() ||
> > > + var->type->is_matrix() ? var->type->vector_elements : 4);
> > > +
> > > + int uniform_offset = var->data.driver_location + offset;
> > > + nir_uniform_offset[uniform_offset] = uniforms;
> > > + offset++;
> > > +
> > > + nir_uniform_driver_location[uniforms] =
var->data.driver_location;
> > > + this->uniforms++;
> > > + }
> > > }
> > >
> > > void
> > > --
> > > 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
