[Mesa-dev] [PATCH 097/133] nir/validate: Ensure that outputs are write-only and inputs are read-only

[Chris Forbes]

Worth pointing out that when tessellation gets ported to NIR, output
reads are a real thing, and can't be lowered away like we do for other
stages (they allow cross-invocation communication, so have to actually
read back from the URB on i965)

On Wed, Jan 7, 2015 at 10:43 AM, Jason Ekstrand <jason at jlekstrand.net> wrote:
>
>
> On Sun, Jan 4, 2015 at 8:48 PM, Connor Abbott <cwabbott0 at gmail.com> wrote:
>>
>> I'm not so sure how I feel about checking that outputs are write-only...
>> eventually we'll want to do lower_input_reads in NIR itself, at which point
>> we'll need to remove that part from the validator. At the same time, for now
>> this is somewhat useful. I'm just not sure if it's worth it (making sure
>> inputs are read-only definitely is, though).
>
>
> Yes, we may want to do that lowering in NIR and I've already written the
> pass.  It's lying around somewhere in my git tree.  However, the only
> producers of NIR that we currently have are GLSL IR which already has the
> pass, and TGSI which already has the outputs-are-write-only restriction.
> For now, I like this bit of validation as it keeps us from breaking other
> passes that depend on this.
>
> Do we want this in the end?  Absoltuely!  We do not want to deal with this
> in nir_lower_variables.  I tried to but it's an absolute pain.  It's much
> easier to just remove the copies.
>
>>
>>
>> On Tue, Dec 16, 2014 at 1:11 AM, Jason Ekstrand <jason at jlekstrand.net>
>> wrote:
>>>
>>> ---
>>>  src/glsl/nir/nir_validate.c | 23 +++++++++++++++++++++++
>>>  1 file changed, 23 insertions(+)
>>>
>>> diff --git a/src/glsl/nir/nir_validate.c b/src/glsl/nir/nir_validate.c
>>> index 80faa15..b8ef802 100644
>>> --- a/src/glsl/nir/nir_validate.c
>>> +++ b/src/glsl/nir/nir_validate.c
>>> @@ -337,6 +337,29 @@ validate_intrinsic_instr(nir_intrinsic_instr *instr,
>>> validate_state *state)
>>>        validate_deref_var(instr->variables[i], state);
>>>     }
>>>
>>> +   switch (instr->intrinsic) {
>>> +   case nir_intrinsic_load_var_vec1:
>>> +   case nir_intrinsic_load_var_vec2:
>>> +   case nir_intrinsic_load_var_vec3:
>>> +   case nir_intrinsic_load_var_vec4:
>>> +      assert(instr->variables[0]->var->data.mode != nir_var_shader_out);
>>> +      break;
>>> +   case nir_intrinsic_store_var_vec1:
>>> +   case nir_intrinsic_store_var_vec2:
>>> +   case nir_intrinsic_store_var_vec3:
>>> +   case nir_intrinsic_store_var_vec4:
>>> +      assert(instr->variables[0]->var->data.mode != nir_var_shader_in &&
>>> +             instr->variables[0]->var->data.mode != nir_var_uniform);
>>> +      break;
>>> +   case nir_intrinsic_copy_var:
>>> +      assert(instr->variables[0]->var->data.mode != nir_var_shader_in &&
>>> +             instr->variables[0]->var->data.mode != nir_var_uniform);
>>> +      assert(instr->variables[1]->var->data.mode != nir_var_shader_out);
>>> +      break;
>>> +   default:
>>> +      break;
>>> +   }
>>> +
>>>     if (instr->has_predicate)
>>>        validate_src(&instr->predicate, state);
>>>  }
>>> --
>>> 2.2.0
>>>
>>> _______________________________________________
>>> mesa-dev mailing list
>>> mesa-dev at lists.freedesktop.org
>>> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
>>
>>
>
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
>