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

[Eric Anholt]

Timothy Arceri <tarceri at itsqueeze.com> writes:

> 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.

7 years later, handling ir location fields doesn't sound so bad any more
:)

> 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.

Awesome.  Let's continue on with this, then!
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 832 bytes
Desc: not available
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20171010/ca7c72b6/attachment.sig>