[Mesa-dev] [PATCH 1/3] nir: Add support for CSE on textures.

[Connor Abbott]

On Tue, Feb 10, 2015 at 1:32 PM, Eric Anholt <eric at anholt.net> wrote:
> Connor Abbott <cwabbott0 at gmail.com> writes:
>
>> On Sat, Feb 7, 2015 at 12:16 AM, Eric Anholt <eric at anholt.net> wrote:
>>> NIR instruction count results on i965:
>>> total instructions in shared programs: 1261954 -> 1261937 (-0.00%)
>>> instructions in affected programs:     455 -> 438 (-3.74%)
>>>
>>> One in yofrankie, two in tropics.  Apparently i965 had also optimized all
>>> of these out anyway.
>>> ---
>>>  src/glsl/nir/nir_opt_cse.c | 43 +++++++++++++++++++++++++++++++++++++++----
>>>  1 file changed, 39 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/src/glsl/nir/nir_opt_cse.c b/src/glsl/nir/nir_opt_cse.c
>>> index b3e9c0d..55dc083 100644
>>> --- a/src/glsl/nir/nir_opt_cse.c
>>> +++ b/src/glsl/nir/nir_opt_cse.c
>>> @@ -80,8 +80,41 @@ nir_instrs_equal(nir_instr *instr1, nir_instr *instr2)
>>>        }
>>>        return true;
>>>     }
>>> -   case nir_instr_type_tex:
>>> -      return false;
>>> +   case nir_instr_type_tex: {
>>> +      nir_tex_instr *tex1 = nir_instr_as_tex(instr1);
>>> +      nir_tex_instr *tex2 = nir_instr_as_tex(instr2);
>>> +
>>> +      if (tex1->op != tex2->op)
>>> +         return false;
>>> +
>>> +      if (tex1->num_srcs != tex2->num_srcs)
>>> +         return false;
>>> +      for (unsigned i = 0; i < tex1->num_srcs; i++) {
>>> +         if (tex1->src[i].src_type != tex2->src[i].src_type ||
>>> +             !nir_srcs_equal(tex1->src[i].src, tex2->src[i].src)) {
>>> +            return false;
>>> +         }
>>> +      }
>>> +
>>> +      if (tex1->coord_components != tex2->coord_components ||
>>> +          tex1->sampler_dim != tex2->sampler_dim ||
>>> +          tex1->is_array != tex2->is_array ||
>>> +          tex1->is_shadow != tex2->is_shadow ||
>>> +          tex1->is_new_style_shadow != tex2->is_new_style_shadow ||
>>> +          memcmp(tex1->const_offset, tex2->const_offset,
>>> +                 sizeof(tex1->const_offset)) != 0 ||
>>> +          tex1->component != tex2->component ||
>>> +         tex1->sampler_index != tex2->sampler_index ||
>>> +         tex1->sampler_array_size != tex2->sampler_array_size) {
>>
>> I think some of these fields are sometimes unused, so you need to
>> split them out and not check them if they're unused. For example,
>> is_new_style_shadow is only used when is_shadow is true, and component
>> isn't used if we're not doing a textureGather (?) op -- I don't
>> remember all the details. That, or we have to have a defined value for
>> these fields when they're unused and document that somewhere.
>
> Oh god, we don't zero-allocate structures.  This is going to be awful.

No, with valgrind it's actually better... you get an error if you
don't know what you're doing rather than silently having it ignored.
These days, zero-allocating structures by default doesn't make much
sense any more.