On Tuesday, June 30, 2015 10:04:47 AM Iago Toral 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;
Oh, right. I think we handle sampler uniforms a bit differently in the vec4 and FS backends, and I was never quite sure why...I know we've had bugs relating to zero-sized uniforms, but I was never quite able to sort them out. This seems fine for now - it keeps the vec4 world doing what it's always done.
signature.asc
Description: This is a digitally signed message part.
_______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
