[Mesa-dev] [PATCH 3/6] nir: add variant of lower_io_to_scalar to be called earlier

[Timothy Arceri]

On 10/10/17 09:31, Timothy Arceri wrote:
> On 10/10/17 09:06, Eric Anholt wrote:
>> Timothy Arceri <tarceri at itsqueeze.com> writes:
>>
>>> This is intended to be called before nir_lower_io() so that we
>>> can do some linking optimisations with the results. It can also
>>> be used with drivers that don't use nir_lower_io() at all such
>>> as RADV.
>>
>>> +static void
>>> +lower_load_to_scalar_early(nir_builder *b, nir_intrinsic_instr *intr,
>>> +                           nir_variable *var, struct hash_table 
>>> *split_inputs,
>>> +                           struct hash_table *split_outputs)
>>> +{
>>> +   b->cursor = nir_before_instr(&intr->instr);
>>> +
>>> +   assert(intr->dest.is_ssa);
>>> +
>>> +   nir_ssa_def *loads[4];
>>> +
>>> +   nir_variable **chan_vars;
>>> +   if (var->data.mode == nir_var_shader_in) {
>>> +      chan_vars = get_channel_variables(split_inputs, var);
>>> +   } else {
>>> +      chan_vars = get_channel_variables(split_outputs, var);
>>> +   }
>>> +
>>> +   for (unsigned i = 0; i < intr->num_components; i++) {
>>> +      nir_variable *chan_var = chan_vars[var->data.location_frac + i];
>>> +      if (!chan_vars[var->data.location_frac + i]) {
>>> +         chan_var = nir_variable_clone(var, b->shader);
>>> +         chan_var->data.location_frac =  var->data.location_frac + i;
>>> +         chan_var->type = glsl_channel_type(chan_var->type);
>>> +
>>> +         chan_vars[var->data.location_frac + i] = chan_var;
>>> +
>>> +         nir_shader_add_variable(b->shader, chan_var);
>>> +      }
>>> +
>>> +      nir_intrinsic_instr *chan_intr =
>>> +         nir_intrinsic_instr_create(b->shader, intr->intrinsic);
>>> +      nir_ssa_dest_init(&chan_intr->instr, &chan_intr->dest,
>>> +                        1, intr->dest.ssa.bit_size, NULL);
>>> +      chan_intr->num_components = 1;
>>> +      chan_intr->variables[0] = nir_deref_var_create(chan_intr, 
>>> chan_var);
>>> +
>>> +      if (intr->variables[0]->deref.child) {
>>> +         chan_intr->variables[0]->deref.child =
>>> +            
>>> &clone_deref_array(nir_deref_as_array(intr->variables[0]->deref.child),
>>> +                               &chan_intr->variables[0]->deref)->deref;
>>> +      }
>>
>> Weird.  I would have expected that we would turn arrays into separate
>> vectors (if possible) before this pass, so there wouldn't be any deref
>> chains.  Has scalarizing arryas, specifically, been useful?
> 
> I don't have any shader-db numbers for this specifically (I guess I 
> could check it out) but I was definitely ending up here via the test 
> suites.

Seems I've looked at it before but forgot. The GLSL IR array splitting 
pass has a comment:

" * This skips uniform/varying arrays, which would need careful
  * handling due to their ir->location fields tying them to the GL API
  * and other shader stages."

So we don't split varying arrays at all there, and there is nothing in 
nir that does it either. I seem to have created a file to do the 
splitting in nir but its empty, I think I wanted to get this pass done 
first.

To answer your question I've spotted shaders in shader-db where removing 
components of an array helps, and also where splitting the array so we 
can remove dead elements would help.