[Mesa-dev] [PATCH 1/8] nir: add varying array splitting pass

[Timothy Arceri]

On 29/11/17 22:27, Nicolai Hähnle wrote:
> On 29.11.2017 12:20, Timothy Arceri wrote:
>> On 29/11/17 21:47, Nicolai Hähnle wrote:
>>> On 21.11.2017 04:28, Timothy Arceri wrote:
>>>> +      nir_ssa_dest_init(&element_intr->instr, &element_intr->dest,
>>>> +                        intr->num_components, 
>>>> intr->dest.ssa.bit_size, NULL);
>>>> +
>>>> +      if (intr->intrinsic == nir_intrinsic_interp_var_at_offset ||
>>>> +          intr->intrinsic == nir_intrinsic_interp_var_at_sample)
>>>> +         nir_src_copy(element_intr->src, intr->src, 
>>>> &element_intr->instr);
>>>
>>> &element_int->src[0], &intr->src[0] for clarity, I think.
> 
> ?
> 
> 
>>>> +
>>>> +      nir_ssa_def_rewrite_uses(&intr->dest.ssa,
>>>> + nir_src_for_ssa(&element_intr->dest.ssa));
>>>> +   } else {
>>>> +      nir_intrinsic_set_write_mask(element_intr, 
>>>> nir_intrinsic_write_mask(intr));
>>>> +      element_intr->src[0] = intr->src[0];
>>>
>>> I'm still not 100% up on my NIR, but doesn't this also need to be a 
>>> nir_src_copy? I think it makes a difference if there's an indirect 
>>> access involved.
>>
>> We don't split indirects so it shouldn't matter here.
> 
> Maybe I'm reading this wrong, but AFAIU this would be for indirects in 
> the source of a nir_intrinsic_store_var. So, storing to a non-indirect 
> output variable, but taking a value from an indirect access to e.g. a 
> temporary array. Those could still exist, right?
> 
> (And even if they can't it might be good to use nir_src_copy anyway for 
> defensive programming.)

Sure makes sense. I'll fix both of these too. Thanks.

> 
> Cheers,
> Nicolai