[Mesa-dev] [PATCH] i965/nir/vec4: fill the type of the dst and src when loading an uniform

[Alejandro Piñeiro]

On 16/09/15 01:39, Jason Ekstrand wrote:
> On Tue, Sep 15, 2015 at 3:23 PM, Alejandro Piñeiro <apinheiro at igalia.com> wrote:
>> Until know, nir_intrinsic_load_uniform was using a default type. This
>           "now"
>> caused some type mismatch between movs and alu operations, so copy
>> propagation optimization was not applied to remove unneeded movs if
>> negate modifier was involved. This was first detected on minus
>> (negate+add) operations with uniforms involved.
>>
>> This patch is equivalent to commit 4de86, but that one only affected
>> inputs, which information, included the type, was already being stored.
>> So this patch includes saving the type on nir_setup_uniforms.
> Can't we just fix copy propagation?  If the copy operation is a dumb
> MOV where there's no source/dest modifiers and the source and
> destination types are the same, then we can copy-propagate regardless
> of whether the type of the MOV matches the type of the thing we're
> copying into.  Because NIR is typeless, we're liable to get type
> mismatches on MOVs and that's just kind of part of life now.  If I
> recall correctly, we did that in the FS backend and it worked out
> pretty well.

Ok, will work on a new patch. Thanks for the feedback.

> --Jason
>
>> Shader DB results (taking into account only vec4):
>>
>> total instructions in shared programs: 1754416 -> 1742146 (-0.70%)
>> instructions in affected programs:     677491 -> 665221 (-1.81%)
>> helped:                                5381
>> HURT:                                  0
>> ---
>>  src/mesa/drivers/dri/i965/brw_vec4.h           | 1 +
>>  src/mesa/drivers/dri/i965/brw_vec4_nir.cpp     | 4 ++++
>>  src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp | 1 +
>>  3 files changed, 6 insertions(+)
>>
>> diff --git a/src/mesa/drivers/dri/i965/brw_vec4.h b/src/mesa/drivers/dri/i965/brw_vec4.h
>> index 01c6e84..f6d2c82 100644
>> --- a/src/mesa/drivers/dri/i965/brw_vec4.h
>> +++ b/src/mesa/drivers/dri/i965/brw_vec4.h
>> @@ -163,6 +163,7 @@ public:
>>     int *uniform_vector_size;
>>     int uniform_array_size; /*< Size of uniform_[vector_]size arrays */
>>     int uniforms;
>> +   brw_reg_type *uniform_type;
>>
>>     src_reg shader_start_time;
>>
>> diff --git a/src/mesa/drivers/dri/i965/brw_vec4_nir.cpp b/src/mesa/drivers/dri/i965/brw_vec4_nir.cpp
>> index 175d92b..0b624fa 100644
>> --- a/src/mesa/drivers/dri/i965/brw_vec4_nir.cpp
>> +++ b/src/mesa/drivers/dri/i965/brw_vec4_nir.cpp
>> @@ -143,6 +143,7 @@ vec4_visitor::nir_setup_uniforms(nir_shader *shader)
>>
>>           assert(uniforms < uniform_array_size);
>>           this->uniform_size[uniforms] = type_size_vec4(var->type);
>> +         this->uniform_type[uniforms] = brw_type_for_base_type(var->type);
>>
>>           if (strncmp(var->name, "gl_", 3) == 0)
>>              nir_setup_builtin_uniform(var);
>> @@ -553,6 +554,9 @@ vec4_visitor::nir_emit_intrinsic(nir_intrinsic_instr *instr)
>>        src = src_reg(dst_reg(UNIFORM, instr->const_index[0]));
>>        src.reg_offset = instr->const_index[1];
>>
>> +      assert(instr->const_index[0] < uniform_array_size);
>> +      dest.type = src.type = this->uniform_type[instr->const_index[0]];
>> +
>>        if (has_indirect) {
>>           src_reg tmp = get_nir_src(instr->src[0], BRW_REGISTER_TYPE_D, 1);
>>           src.reladdr = new(mem_ctx) src_reg(tmp);
>> diff --git a/src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp b/src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp
>> index 0465770..8e00612 100644
>> --- a/src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp
>> +++ b/src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp
>> @@ -3747,6 +3747,7 @@ vec4_visitor::vec4_visitor(const struct brw_compiler *compiler,
>>
>>     this->uniform_size = rzalloc_array(mem_ctx, int, this->uniform_array_size);
>>     this->uniform_vector_size = rzalloc_array(mem_ctx, int, this->uniform_array_size);
>> +   this->uniform_type = rzalloc_array(mem_ctx, brw_reg_type, this->uniform_array_size);
>>  }
>>
>>  vec4_visitor::~vec4_visitor()
>> --
>> 2.1.4
>>
>> _______________________________________________
>> mesa-dev mailing list
>> mesa-dev at lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/mesa-dev

-- 
Alejandro Piñeiro (apinheiro at igalia.com)