[Mesa-dev] [PATCH 09/78] i965/nir/vec4: Add shader function implementation

[Eduardo Lima Mitev]

On 06/30/2015 01:47 AM, Jason Ekstrand wrote:
> On Fri, Jun 26, 2015 at 1:06 AM, Eduardo Lima Mitev <elima at igalia.com> wrote:
>> It basically allocates registers local to a function in a nir_locals map,
>> then emits all its control-flow blocks.
>>
>> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=89580
>> ---
>>  src/mesa/drivers/dri/i965/brw_vec4.h       |  1 +
>>  src/mesa/drivers/dri/i965/brw_vec4_nir.cpp | 12 +++++++++++-
>>  2 files changed, 12 insertions(+), 1 deletion(-)
>>
>> diff --git a/src/mesa/drivers/dri/i965/brw_vec4.h b/src/mesa/drivers/dri/i965/brw_vec4.h
>> index e531d60..d837d90 100644
>> --- a/src/mesa/drivers/dri/i965/brw_vec4.h
>> +++ b/src/mesa/drivers/dri/i965/brw_vec4.h
>> @@ -414,6 +414,7 @@ public:
>>     virtual dst_reg *make_reg_for_system_value(int location,
>>                                                const glsl_type *type) = 0;
>>
>> +   dst_reg *nir_locals;
>>     src_reg *nir_inputs;
>>     int *nir_outputs;
>>     brw_reg_type *nir_output_types;
>> diff --git a/src/mesa/drivers/dri/i965/brw_vec4_nir.cpp b/src/mesa/drivers/dri/i965/brw_vec4_nir.cpp
>> index 6c2a046..36c9dc0 100644
>> --- a/src/mesa/drivers/dri/i965/brw_vec4_nir.cpp
>> +++ b/src/mesa/drivers/dri/i965/brw_vec4_nir.cpp
>> @@ -274,7 +274,17 @@ vec4_visitor::nir_setup_builtin_uniform(nir_variable *var)
>>  void
>>  vec4_visitor::nir_emit_impl(nir_function_impl *impl)
>>  {
>> -   /* @TODO: Not yet implemented */
>> +   nir_locals = ralloc_array(mem_ctx, dst_reg, impl->reg_alloc);
>> +
>> +   foreach_list_typed(nir_register, reg, node, &impl->registers) {
>> +      unsigned array_elems =
>> +         reg->num_array_elems == 0 ? 1 : reg->num_array_elems;
>> +      unsigned size = array_elems * reg->num_components;
> 
> I don't think you need to do this multiply.  The vec4 backend already
> works in terms of vec4's.  It isn't hurting anything because register
> splitting is just chopping off the extra.  Still, no reason to make it
> bigger than needed.
> 

Right. This is one of the first chunks we copy/pasted from fs_nir and
never really revisited after having a better understanding of the vec4
vs. scalar differences.

I just tested it on piglit and confirm that we don't need the multiply.

>> +
>> +      nir_locals[reg->index] = dst_reg(GRF, alloc.allocate(size));
>> +   }
>> +
>> +   nir_emit_cf_list(&impl->body);
>>  }
>>
>>  void
>> --