[Mesa-dev] [PATCH 19/78] nir/nir_lower_io: Add vec4 support
Iago Toral
itoral at igalia.com
Fri Jul 3 00:58:03 PDT 2015
On Thu, 2015-07-02 at 09:31 +0200, Iago Toral wrote:
> 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.
Notice that this patch already changes how assign_var_locations works
(that calls type_size which is the one thing affected by the is_scalar
flag). The mappings you saw in the implementations of the input and
output intrinsics stopped being necessary with this patch and we will
just remove them, it just happened that I fixed it for uniforms and did
not realize that the same situation affected inputs and outputs, but we
just confirmed that with this patch the indirections are not needed
there either.
So, as things stand now, the changes in this patch are the only changes
we need to nir_lower_io for vec4 shaders, at least for now. Knowing
that, are you okay with this implementation or would you rather change
it to have separate functions for vec4 and scalar?
Iago
> > 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>
> > >
> > > 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 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
More information about the mesa-dev
mailing list