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

[Jason Ekstrand]

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
>>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20150106/d8ca62f6/attachment.html>