Connor Abbott <[email protected]> writes: > On Thu, Dec 18, 2014 at 2:01 AM, Eric Anholt <[email protected]> wrote: >> Jason Ekstrand <[email protected]> writes: >> >>> From: Connor Abbott <[email protected]> >>> >>> This is similar to ir_validate.cpp. >>> >>> v2: Jason Ekstrand <[email protected]>: >>> 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.
OK, so it seems like this validation needs to be dropped.
>>> +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.
I must have skimmed right past that.
signature.asc
Description: PGP signature
_______________________________________________ mesa-dev mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/mesa-dev
