On Tue, 2015-06-30 at 11:32 -0700, Jason Ekstrand wrote: > I'm not sure what I think about adding an is_scalar flag vs. having > _scalar and _vec4 versions of each function. My feeling is that once > we tweak assign_var_locations as I mentioned for vec4 outputs, we'll > find that we want to have them separate. The big thing here is that > I'd rather have _vec4 and _scalar versions internally call the same > function than have a general-looking function that switches inside on > is_scalar and does two completely different things. So if we can > share code like this for all of them, is_scalar is totally fine. If > we start having divergence, different scalar/vec4 functions is > probably better. > --Jason
Ok, let's see how the changes to vec4 outputs affects this. If we end up needing more changes than just the kind of type_size() function we need to call then I'll look into having separate paths for scalar and vec4. Iago > On Fri, Jun 26, 2015 at 1:06 AM, Eduardo Lima Mitev <el...@igalia.com> wrote: > > From: Iago Toral Quiroga <ito...@igalia.com> > > > > The current implementation operates in scalar mode only, so add a vec4 > > mode where types are padded to vec4 sizes. > > > > This will be useful in the i965 driver for its vec4 nir backend > > (and possbly other drivers that have vec4-based shaders). > > > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=89580 > > --- > > src/glsl/nir/nir.h | 18 ++++---- > > src/glsl/nir/nir_lower_io.c | 87 > > +++++++++++++++++++++++++++++-------- > > src/mesa/drivers/dri/i965/brw_nir.c | 18 +++++--- > > 3 files changed, 90 insertions(+), 33 deletions(-) > > > > diff --git a/src/glsl/nir/nir.h b/src/glsl/nir/nir.h > > index 697d37e..334abbc 100644 > > --- a/src/glsl/nir/nir.h > > +++ b/src/glsl/nir/nir.h > > @@ -1640,14 +1640,16 @@ void nir_lower_global_vars_to_local(nir_shader > > *shader); > > > > void nir_lower_locals_to_regs(nir_shader *shader); > > > > -void nir_assign_var_locations_scalar(struct exec_list *var_list, > > - unsigned *size); > > -void nir_assign_var_locations_scalar_direct_first(nir_shader *shader, > > - struct exec_list > > *var_list, > > - unsigned *direct_size, > > - unsigned *size); > > - > > -void nir_lower_io(nir_shader *shader); > > +void nir_assign_var_locations(struct exec_list *var_list, > > + unsigned *size, > > + bool is_scalar); > > +void nir_assign_var_locations_direct_first(nir_shader *shader, > > + struct exec_list *var_list, > > + unsigned *direct_size, > > + unsigned *size, > > + bool is_scalar); > > + > > +void nir_lower_io(nir_shader *shader, bool is_scalar); > > > > void nir_lower_vars_to_ssa(nir_shader *shader); > > > > diff --git a/src/glsl/nir/nir_lower_io.c b/src/glsl/nir/nir_lower_io.c > > index 6761d5b..3ecd386 100644 > > --- a/src/glsl/nir/nir_lower_io.c > > +++ b/src/glsl/nir/nir_lower_io.c > > @@ -29,19 +29,56 @@ > > /* > > * This lowering pass converts references to input/output variables with > > * loads/stores to actual input/output intrinsics. > > - * > > - * NOTE: This pass really only works for scalar backends at the moment due > > - * to the way it packes the input/output data. > > */ > > > > #include "nir.h" > > > > struct lower_io_state { > > void *mem_ctx; > > + bool is_scalar; > > }; > > > > +static int > > +type_size_vec4(const struct glsl_type *type) > > +{ > > + unsigned int i; > > + int size; > > + > > + switch (glsl_get_base_type(type)) { > > + case GLSL_TYPE_UINT: > > + case GLSL_TYPE_INT: > > + case GLSL_TYPE_FLOAT: > > + case GLSL_TYPE_BOOL: > > + if (glsl_type_is_matrix(type)) { > > + return glsl_get_matrix_columns(type); > > + } else { > > + return 1; > > + } > > + case GLSL_TYPE_ARRAY: > > + return type_size_vec4(glsl_get_array_element(type)) * > > glsl_get_length(type); > > + case GLSL_TYPE_STRUCT: > > + size = 0; > > + for (i = 0; i < glsl_get_length(type); i++) { > > + size += type_size_vec4(glsl_get_struct_field(type, i)); > > + } > > + return size; > > + case GLSL_TYPE_SAMPLER: > > + return 0; > > + case GLSL_TYPE_ATOMIC_UINT: > > + return 0; > > + case GLSL_TYPE_IMAGE: > > + case GLSL_TYPE_VOID: > > + case GLSL_TYPE_DOUBLE: > > + case GLSL_TYPE_ERROR: > > + case GLSL_TYPE_INTERFACE: > > + unreachable("not reached"); > > + } > > + > > + return 0; > > +} > > + > > static unsigned > > -type_size(const struct glsl_type *type) > > +type_size_scalar(const struct glsl_type *type) > > { > > unsigned int size, i; > > > > @@ -52,11 +89,11 @@ type_size(const struct glsl_type *type) > > case GLSL_TYPE_BOOL: > > return glsl_get_components(type); > > case GLSL_TYPE_ARRAY: > > - return type_size(glsl_get_array_element(type)) * > > glsl_get_length(type); > > + return type_size_scalar(glsl_get_array_element(type)) * > > glsl_get_length(type); > > case GLSL_TYPE_STRUCT: > > size = 0; > > for (i = 0; i < glsl_get_length(type); i++) { > > - size += type_size(glsl_get_struct_field(type, i)); > > + size += type_size_scalar(glsl_get_struct_field(type, i)); > > } > > return size; > > case GLSL_TYPE_SAMPLER: > > @@ -76,8 +113,17 @@ type_size(const struct glsl_type *type) > > return 0; > > } > > > > +static unsigned > > +type_size(const struct glsl_type *type, bool is_scalar) > > +{ > > + if (is_scalar) > > + return type_size_scalar(type); > > + else > > + return type_size_vec4(type); > > +} > > + > > void > > -nir_assign_var_locations_scalar(struct exec_list *var_list, unsigned *size) > > +nir_assign_var_locations(struct exec_list *var_list, unsigned *size, bool > > is_scalar) > > { > > unsigned location = 0; > > > > @@ -90,7 +136,7 @@ nir_assign_var_locations_scalar(struct exec_list > > *var_list, unsigned *size) > > continue; > > > > var->data.driver_location = location; > > - location += type_size(var->type); > > + location += type_size(var->type, is_scalar); > > } > > > > *size = location; > > @@ -136,10 +182,11 @@ mark_indirect_uses_block(nir_block *block, void > > *void_state) > > * assigns locations to variables that are used indirectly. > > */ > > void > > -nir_assign_var_locations_scalar_direct_first(nir_shader *shader, > > - struct exec_list *var_list, > > - unsigned *direct_size, > > - unsigned *size) > > +nir_assign_var_locations_direct_first(nir_shader *shader, > > + struct exec_list *var_list, > > + unsigned *direct_size, > > + unsigned *size, > > + bool is_scalar) > > { > > struct set *indirect_set = _mesa_set_create(NULL, _mesa_hash_pointer, > > _mesa_key_pointer_equal); > > @@ -160,7 +207,7 @@ nir_assign_var_locations_scalar_direct_first(nir_shader > > *shader, > > continue; > > > > var->data.driver_location = location; > > - location += type_size(var->type); > > + location += type_size(var->type, is_scalar); > > } > > > > *direct_size = location; > > @@ -173,7 +220,7 @@ nir_assign_var_locations_scalar_direct_first(nir_shader > > *shader, > > continue; > > > > var->data.driver_location = location; > > - location += type_size(var->type); > > + location += type_size(var->type, is_scalar); > > } > > > > *size = location; > > @@ -195,7 +242,7 @@ get_io_offset(nir_deref_var *deref, nir_instr *instr, > > nir_src *indirect, > > > > if (tail->deref_type == nir_deref_type_array) { > > nir_deref_array *deref_array = nir_deref_as_array(tail); > > - unsigned size = type_size(tail->type); > > + unsigned size = type_size(tail->type, state->is_scalar); > > > > base_offset += size * deref_array->base_offset; > > > > @@ -237,7 +284,8 @@ get_io_offset(nir_deref_var *deref, nir_instr *instr, > > nir_src *indirect, > > nir_deref_struct *deref_struct = nir_deref_as_struct(tail); > > > > for (unsigned i = 0; i < deref_struct->index; i++) > > - base_offset += type_size(glsl_get_struct_field(parent_type, > > i)); > > + base_offset += type_size(glsl_get_struct_field(parent_type, i), > > + state->is_scalar); > > } > > } > > > > @@ -350,11 +398,12 @@ nir_lower_io_block(nir_block *block, void *void_state) > > } > > > > static void > > -nir_lower_io_impl(nir_function_impl *impl) > > +nir_lower_io_impl(nir_function_impl *impl, bool is_scalar) > > { > > struct lower_io_state state; > > > > state.mem_ctx = ralloc_parent(impl); > > + state.is_scalar = is_scalar; > > > > nir_foreach_block(impl, nir_lower_io_block, &state); > > > > @@ -363,10 +412,10 @@ nir_lower_io_impl(nir_function_impl *impl) > > } > > > > void > > -nir_lower_io(nir_shader *shader) > > +nir_lower_io(nir_shader *shader, bool is_scalar) > > { > > nir_foreach_overload(shader, overload) { > > if (overload->impl) > > - nir_lower_io_impl(overload->impl); > > + nir_lower_io_impl(overload->impl, is_scalar); > > } > > } > > diff --git a/src/mesa/drivers/dri/i965/brw_nir.c > > b/src/mesa/drivers/dri/i965/brw_nir.c > > index 885c49d..002ae3e 100644 > > --- a/src/mesa/drivers/dri/i965/brw_nir.c > > +++ b/src/mesa/drivers/dri/i965/brw_nir.c > > @@ -22,6 +22,7 @@ > > */ > > > > #include "brw_nir.h" > > +#include "brw_shader.h" > > #include "glsl/glsl_parser_extras.h" > > #include "glsl/nir/glsl_to_nir.h" > > #include "program/prog_to_nir.h" > > @@ -70,6 +71,9 @@ brw_create_nir(struct brw_context *brw, > > bool debug_enabled = INTEL_DEBUG & > > intel_debug_flag_for_shader_stage(stage); > > nir_shader *nir; > > > > + bool is_scalar = > > + brw_compiler_is_scalar_shader_stage(brw->intelScreen->compiler, > > stage); > > + > > /* First, lower the GLSL IR or Mesa IR to NIR */ > > if (shader_prog) { > > nir = glsl_to_nir(shader, options); > > @@ -100,13 +104,15 @@ brw_create_nir(struct brw_context *brw, > > /* Get rid of split copies */ > > nir_optimize(nir); > > > > - nir_assign_var_locations_scalar_direct_first(nir, &nir->uniforms, > > - &nir->num_direct_uniforms, > > - &nir->num_uniforms); > > - nir_assign_var_locations_scalar(&nir->inputs, &nir->num_inputs); > > - nir_assign_var_locations_scalar(&nir->outputs, &nir->num_outputs); > > + nir_assign_var_locations_direct_first(nir, &nir->uniforms, > > + &nir->num_direct_uniforms, > > + &nir->num_uniforms, > > + is_scalar); > > + nir_assign_var_locations(&nir->inputs, &nir->num_inputs, is_scalar); > > + nir_assign_var_locations(&nir->outputs, &nir->num_outputs, is_scalar); > > + > > + nir_lower_io(nir, is_scalar); > > > > - nir_lower_io(nir); > > nir_validate_shader(nir); > > > > nir_remove_dead_variables(nir); > > -- > > 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
