[Mesa-dev] [PATCH 4/8] nir: Pass a type_size() function pointer into nir_lower_io().

Jason Ekstrand jason at jlekstrand.net
Tue Aug 18 20:25:22 PDT 2015


On Mon, Aug 17, 2015 at 4:44 PM, Jason Ekstrand <jason at jlekstrand.net> wrote:
> The first four are
>
> Reviewed-by: Jason Ekstrand <jason.ekstrand at intel.com>

Before you push any of these, this patch (number 4) breaks
image_load_store in a kind-of obscure way.  I just sent a patch to fix
it up.  That patch needs to be applied before this one.

>
> I'll get to the others later.
>
> On Mon, Aug 17, 2015 at 4:07 PM, Kenneth Graunke <kenneth at whitecape.org> wrote:
>> Previously, there were four type_size() functions in play - the i965
>> compiler backend defined scalar and vec4 type_size() functions, and
>> nir_lower_io contained its own similar functions.
>>
>> In fact, the i965 driver used nir_lower_io() and then looped over the
>> components using its own type_size - meaning both were in play.  The
>> two are /basically/ the same, but not exactly in obscure cases like
>> subroutines and images.
>>
>> This patch removes nir_lower_io's functions, and instead makes the
>> driver supply a function pointer.  This gives the driver ultimate
>> flexibility in deciding how it wants to count things, reduces code
>> duplication, and improves consistency.
>>
>> Signed-off-by: Kenneth Graunke <kenneth at whitecape.org>
>> ---
>>  src/glsl/nir/nir.h                  |   8 +--
>>  src/glsl/nir/nir_lower_io.c         | 118 +++++-------------------------------
>>  src/mesa/drivers/dri/i965/brw_nir.c |  16 +++--
>>  3 files changed, 30 insertions(+), 112 deletions(-)
>>
>> diff --git a/src/glsl/nir/nir.h b/src/glsl/nir/nir.h
>> index 222a219..8df0de9 100644
>> --- a/src/glsl/nir/nir.h
>> +++ b/src/glsl/nir/nir.h
>> @@ -1639,15 +1639,15 @@ void nir_lower_locals_to_regs(nir_shader *shader);
>>
>>  void nir_assign_var_locations(struct exec_list *var_list,
>>                                unsigned *size,
>> -                              bool is_scalar);
>> +                              int (*type_size)(const struct glsl_type *));
>>  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);
>> +                                           int (*type_size)(const struct glsl_type *));
>>
>> +void nir_lower_io(nir_shader *shader,
>> +                  int (*type_size)(const struct glsl_type *));
>>  void nir_lower_vars_to_ssa(nir_shader *shader);
>>
>>  void nir_remove_dead_variables(nir_shader *shader);
>> diff --git a/src/glsl/nir/nir_lower_io.c b/src/glsl/nir/nir_lower_io.c
>> index d33aefe..15a4edc 100644
>> --- a/src/glsl/nir/nir_lower_io.c
>> +++ b/src/glsl/nir/nir_lower_io.c
>> @@ -37,99 +37,12 @@
>>  struct lower_io_state {
>>     nir_builder builder;
>>     void *mem_ctx;
>> -   bool is_scalar;
>> +   int (*type_size)(const struct glsl_type *type);
>>  };
>>
>> -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_SUBROUTINE:
>> -      return 1;
>> -   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_scalar(const struct glsl_type *type)
>> -{
>> -   unsigned int size, i;
>> -
>> -   switch (glsl_get_base_type(type)) {
>> -   case GLSL_TYPE_UINT:
>> -   case GLSL_TYPE_INT:
>> -   case GLSL_TYPE_FLOAT:
>> -   case GLSL_TYPE_BOOL:
>> -      return glsl_get_components(type);
>> -   case GLSL_TYPE_ARRAY:
>> -      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_scalar(glsl_get_struct_field(type, i));
>> -      }
>> -      return size;
>> -   case GLSL_TYPE_SUBROUTINE:
>> -      return 1;
>> -   case GLSL_TYPE_SAMPLER:
>> -      return 0;
>> -   case GLSL_TYPE_ATOMIC_UINT:
>> -      return 0;
>> -   case GLSL_TYPE_INTERFACE:
>> -      return 0;
>> -   case GLSL_TYPE_IMAGE:
>> -      return 0;
>> -   case GLSL_TYPE_VOID:
>> -   case GLSL_TYPE_ERROR:
>> -   case GLSL_TYPE_DOUBLE:
>> -      unreachable("not reached");
>> -   }
>> -
>> -   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(struct exec_list *var_list, unsigned *size, bool is_scalar)
>> +nir_assign_var_locations(struct exec_list *var_list, unsigned *size,
>> +                         int (*type_size)(const struct glsl_type *))
>>  {
>>     unsigned location = 0;
>>
>> @@ -143,7 +56,7 @@ nir_assign_var_locations(struct exec_list *var_list, unsigned *size, bool is_sca
>>           continue;
>>
>>        var->data.driver_location = location;
>> -      location += type_size(var->type, is_scalar);
>> +      location += type_size(var->type);
>>     }
>>
>>     *size = location;
>> @@ -193,7 +106,7 @@ nir_assign_var_locations_direct_first(nir_shader *shader,
>>                                        struct exec_list *var_list,
>>                                        unsigned *direct_size,
>>                                        unsigned *size,
>> -                                      bool is_scalar)
>> +                                      int (*type_size)(const struct glsl_type *))
>>  {
>>     struct set *indirect_set = _mesa_set_create(NULL, _mesa_hash_pointer,
>>                                                 _mesa_key_pointer_equal);
>> @@ -215,7 +128,7 @@ nir_assign_var_locations_direct_first(nir_shader *shader,
>>           continue;
>>
>>        var->data.driver_location = location;
>> -      location += type_size(var->type, is_scalar);
>> +      location += type_size(var->type);
>>     }
>>
>>     *direct_size = location;
>> @@ -229,7 +142,7 @@ nir_assign_var_locations_direct_first(nir_shader *shader,
>>           continue;
>>
>>        var->data.driver_location = location;
>> -      location += type_size(var->type, is_scalar);
>> +      location += type_size(var->type);
>>     }
>>
>>     *size = location;
>> @@ -254,7 +167,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, state->is_scalar);
>> +         unsigned size = state->type_size(tail->type);
>>
>>           base_offset += size * deref_array->base_offset;
>>
>> @@ -275,9 +188,10 @@ get_io_offset(nir_deref_var *deref, nir_instr *instr, nir_src *indirect,
>>        } else if (tail->deref_type == nir_deref_type_struct) {
>>           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),
>> -                                     state->is_scalar);
>> +         for (unsigned i = 0; i < deref_struct->index; i++) {
>> +            base_offset +=
>> +               state->type_size(glsl_get_struct_field(parent_type, i));
>> +         }
>>        }
>>     }
>>
>> @@ -395,13 +309,13 @@ nir_lower_io_block(nir_block *block, void *void_state)
>>  }
>>
>>  static void
>> -nir_lower_io_impl(nir_function_impl *impl, bool is_scalar)
>> +nir_lower_io_impl(nir_function_impl *impl, int(*type_size)(const struct glsl_type *))
>>  {
>>     struct lower_io_state state;
>>
>>     nir_builder_init(&state.builder, impl);
>>     state.mem_ctx = ralloc_parent(impl);
>> -   state.is_scalar = is_scalar;
>> +   state.type_size = type_size;
>>
>>     nir_foreach_block(impl, nir_lower_io_block, &state);
>>
>> @@ -410,10 +324,10 @@ nir_lower_io_impl(nir_function_impl *impl, bool is_scalar)
>>  }
>>
>>  void
>> -nir_lower_io(nir_shader *shader, bool is_scalar)
>> +nir_lower_io(nir_shader *shader, int(*type_size)(const struct glsl_type *))
>>  {
>>     nir_foreach_overload(shader, overload) {
>>        if (overload->impl)
>> -         nir_lower_io_impl(overload->impl, is_scalar);
>> +         nir_lower_io_impl(overload->impl, type_size);
>>     }
>>  }
>> diff --git a/src/mesa/drivers/dri/i965/brw_nir.c b/src/mesa/drivers/dri/i965/brw_nir.c
>> index b5788fa..3d04363 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"
>> @@ -113,19 +114,22 @@ brw_create_nir(struct brw_context *brw,
>>        nir_assign_var_locations_direct_first(nir, &nir->uniforms,
>>                                              &nir->num_direct_uniforms,
>>                                              &nir->num_uniforms,
>> -                                            is_scalar);
>> -      nir_assign_var_locations(&nir->outputs, &nir->num_outputs, is_scalar);
>> +                                            type_size_scalar);
>> +      nir_assign_var_locations(&nir->inputs, &nir->num_inputs, type_size_scalar);
>> +      nir_assign_var_locations(&nir->outputs, &nir->num_outputs, type_size_scalar);
>> +      nir_lower_io(nir, type_size_scalar);
>>     } else {
>>        nir_assign_var_locations(&nir->uniforms,
>>                                 &nir->num_uniforms,
>> -                               is_scalar);
>> +                               type_size_vec4);
>> +
>> +      nir_assign_var_locations(&nir->inputs, &nir->num_inputs, type_size_vec4);
>>
>>        foreach_list_typed(nir_variable, var, node, &nir->outputs)
>>           var->data.driver_location = var->data.location;
>> -   }
>> -   nir_assign_var_locations(&nir->inputs, &nir->num_inputs, is_scalar);
>>
>> -   nir_lower_io(nir, is_scalar);
>> +      nir_lower_io(nir, type_size_vec4);
>> +   }
>>
>>     nir_validate_shader(nir);
>>
>> --
>> 2.5.0
>>
>> _______________________________________________
>> 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