[Mesa-dev] [PATCH 02/10] i965/vec4: Get rid of the nir_inputs array

[Jason Ekstrand]

On Tue, Dec 1, 2015 at 11:56 PM, Kenneth Graunke <kenneth at whitecape.org> wrote:
> On Wednesday, November 25, 2015 08:55:54 PM Jason Ekstrand wrote:
>> It's not really buying us anything at this point.  It's just a way of
>> remapping one offset namespace onto another.  We can just use the location
>> namespace the whole way through.
>> ---
>>  src/mesa/drivers/dri/i965/brw_nir.c        | 28 ++++++++++++----------------
>>  src/mesa/drivers/dri/i965/brw_vec4.h       |  2 --
>>  src/mesa/drivers/dri/i965/brw_vec4_nir.cpp | 23 +----------------------
>>  3 files changed, 13 insertions(+), 40 deletions(-)
>>
>> diff --git a/src/mesa/drivers/dri/i965/brw_nir.c b/src/mesa/drivers/dri/i965/brw_nir.c
>> index 91358d8..bf3bc2d 100644
>> --- a/src/mesa/drivers/dri/i965/brw_nir.c
>> +++ b/src/mesa/drivers/dri/i965/brw_nir.c
>> @@ -62,13 +62,6 @@ brw_nir_lower_inputs(nir_shader *nir,
>>  {
>>     switch (nir->stage) {
>>     case MESA_SHADER_VERTEX:
>> -      /* For now, leave the vec4 backend doing the old method. */
>> -      if (!is_scalar) {
>> -         nir_assign_var_locations(&nir->inputs, &nir->num_inputs,
>> -                                  type_size_vec4);
>> -         break;
>> -      }
>> -
>>        /* Start with the location of the variable's base. */
>>        foreach_list_typed(nir_variable, var, node, &nir->inputs) {
>>           var->data.driver_location = var->data.location;
>> @@ -80,15 +73,18 @@ brw_nir_lower_inputs(nir_shader *nir,
>>         */
>>        nir_lower_io(nir, nir_var_shader_in, type_size_vec4);
>>
>> -      /* Finally, translate VERT_ATTRIB_* values into the actual registers.
>> -       *
>> -       * Note that we can use nir->info.inputs_read instead of key->inputs_read
>> -       * since the two are identical aside from Gen4-5 edge flag differences.
>> -       */
>> -      GLbitfield64 inputs_read = nir->info.inputs_read;
>> -      nir_foreach_overload(nir, overload) {
>> -         if (overload->impl) {
>> -            nir_foreach_block(overload->impl, remap_vs_attrs, &inputs_read);
>> +      if (is_scalar) {
>> +         /* Finally, translate VERT_ATTRIB_* values into the actual registers.
>> +          *
>> +          * Note that we can use nir->info.inputs_read instead of
>> +          * key->inputs_read since the two are identical aside from Gen4-5
>> +          * edge flag differences.
>> +          */
>> +         GLbitfield64 inputs_read = nir->info.inputs_read;
>> +         nir_foreach_overload(nir, overload) {
>> +            if (overload->impl) {
>> +               nir_foreach_block(overload->impl, remap_vs_attrs, &inputs_read);
>> +            }
>>           }
>>        }
>>        break;
>> diff --git a/src/mesa/drivers/dri/i965/brw_vec4.h b/src/mesa/drivers/dri/i965/brw_vec4.h
>> index 3f67432..a168250 100644
>> --- a/src/mesa/drivers/dri/i965/brw_vec4.h
>> +++ b/src/mesa/drivers/dri/i965/brw_vec4.h
>> @@ -327,7 +327,6 @@ public:
>>     bool is_high_sampler(src_reg sampler);
>>
>>     virtual void emit_nir_code();
>> -   virtual void nir_setup_inputs();
>>     virtual void nir_setup_uniforms();
>>     virtual void nir_setup_system_value_intrinsic(nir_intrinsic_instr *instr);
>>     virtual void nir_setup_system_values();
>> @@ -360,7 +359,6 @@ public:
>>
>>     dst_reg *nir_locals;
>>     dst_reg *nir_ssa_values;
>> -   src_reg *nir_inputs;
>>     dst_reg *nir_system_values;
>>
>>  protected:
>> diff --git a/src/mesa/drivers/dri/i965/brw_vec4_nir.cpp b/src/mesa/drivers/dri/i965/brw_vec4_nir.cpp
>> index c777acf..96787db 100644
>> --- a/src/mesa/drivers/dri/i965/brw_vec4_nir.cpp
>> +++ b/src/mesa/drivers/dri/i965/brw_vec4_nir.cpp
>> @@ -35,9 +35,6 @@ namespace brw {
>>  void
>>  vec4_visitor::emit_nir_code()
>>  {
>> -   if (nir->num_inputs > 0)
>> -      nir_setup_inputs();
>> -
>>     if (nir->num_uniforms > 0)
>>        nir_setup_uniforms();
>>
>> @@ -118,24 +115,6 @@ vec4_visitor::nir_setup_system_values()
>>  }
>>
>>  void
>> -vec4_visitor::nir_setup_inputs()
>> -{
>> -   nir_inputs = ralloc_array(mem_ctx, src_reg, nir->num_inputs);
>> -   for (unsigned i = 0; i < nir->num_inputs; i++) {
>> -      nir_inputs[i] = src_reg();
>> -   }
>> -
>> -   nir_foreach_variable(var, &nir->inputs) {
>> -      int offset = var->data.driver_location;
>> -      unsigned size = type_size_vec4(var->type);
>> -      for (unsigned i = 0; i < size; i++) {
>> -         src_reg src = src_reg(ATTR, var->data.location + i, var->type);
>> -         nir_inputs[offset + i] = src;
>> -      }
>> -   }
>> -}
>> -
>> -void
>>  vec4_visitor::nir_setup_uniforms()
>>  {
>>     uniforms = nir->num_uniforms;
>> @@ -399,7 +378,7 @@ vec4_visitor::nir_emit_intrinsic(nir_intrinsic_instr *instr)
>>        /* fallthrough */
>>     case nir_intrinsic_load_input: {
>>        int offset = instr->const_index[0];
>> -      src = nir_inputs[offset];
>> +      src = src_reg(ATTR, offset, glsl_type::uvec4_type);
>>
>>        if (has_indirect) {
>>           dest.reladdr = new(mem_ctx) src_reg(get_nir_src(instr->src[0],
>>
>
> The one thing this was buying us is that we used the proper type of the
> variable instead of defaulting to UD.  I'd really like to (eventually)
> include a type in load intrinsics so we can load using the native type.
>
> Does this affect shader-db?  If not, patches 1-2 are

Just ran it.  No shader-db changes.

> Reviewed-by: Kenneth Graunke <kenneth at whitecape.org>

Thanks!  I'll go ahead and push 1, 2, and 2.5.