On Thu, Dec 18, 2014 at 2:01 AM, Eric Anholt <e...@anholt.net> wrote:
> Jason Ekstrand <ja...@jlekstrand.net> writes:
>
>> From: Connor Abbott <connor.abb...@intel.com>
>>
>> This is similar to ir_validate.cpp.
>>
>> v2: Jason Ekstrand <jason.ekstr...@intel.com>:
>> whitespace fixes
>
> I have again not reviewed the control flow bits. Couple of questions I
> had, though:
>
>> +static void
>> +validate_var_use(nir_variable *var, validate_state *state)
>> +{
>> + if (var->data.mode == nir_var_local) {
>> + struct hash_entry *entry =
>> + _mesa_hash_table_search(state->var_defs, _mesa_hash_pointer(var),
>> + var);
>> +
>> + assert(entry);
>> + assert((nir_function_impl *) entry->data == state->impl);
>> + }
>> +}
>
> Is there guaranteed to be a def of a local variable before a use? It
> would be undefined execution behavior, but not assertion failure
> quality, right?
Yes, that's correct - there are no guarantees about this for variables
and registers. For SSA values, the definition should always dominate
the use (see the TODO about that) because a lot of SSA algorithms
assume that, so we model the use-before-def case by pointing the use
to a nir_ssa_undef_instr.
>
>> +static void
>> +postvalidate_reg_decl(nir_register *reg, validate_state *state)
>> +{
>> + struct hash_entry *entry = _mesa_hash_table_search(state->regs,
>> +
>> _mesa_hash_pointer(reg),
>> + reg);
>> +
>> + reg_validate_state *reg_state = (reg_validate_state *) entry->data;
>> +
>> + if (reg_state->uses->entries != reg->uses->entries) {
>> + printf("extra entries in register uses:\n");
>> + struct set_entry *entry;
>> + set_foreach(reg->uses, entry) {
>> + struct set_entry *entry2 =
>> + _mesa_set_search(reg_state->uses,
>> _mesa_hash_pointer(entry->key),
>> + entry->key);
>> +
>> + if (entry2 == NULL) {
>> + printf("%p\n", entry->key);
>> + }
>> + }
>> +
>> + abort();
>> + }
>> +
>> + if (reg_state->defs->entries != reg->defs->entries) {
>> + printf("extra entries in register defs:\n");
>> + struct set_entry *entry;
>> + set_foreach(reg->defs, entry) {
>> + struct set_entry *entry2 =
>> + _mesa_set_search(reg_state->defs,
>> _mesa_hash_pointer(entry->key),
>> + entry->key);
>> +
>> + if (entry2 == NULL) {
>> + printf("%p\n", entry->key);
>> + }
>> + }
>
> Couldn't these failures go the other way and there be, for example,
> defs that weren't tracked in the reg?
>
> (Not necessarily important to fix, since you'll at least get the
> abort())
Yeah, the point here is that we've already validated that all the
actual definitions (i.e. everything in reg_state->defs) are already in
reg->defs, so once we've gotten to this point the only possible reason
for them not being the same is that reg->defs has extra entries, which
we check for here.
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
