<div dir="ltr"><div>I did a quick skim. LGTM. Series is </div><div>Acked-by: Jason Ekstrand <<a href="mailto:jason@jlekstrand.net">jason@jlekstrand.net</a>> </div></div><div class="gmail_extra"> <div class="gmail_quote">On Mon, May 16, 2016 at 4:46 PM, Connor Abbott <<a href="mailto:cwabbott0@gmail.com" target="_blank">cwabbott0@gmail.com</a>> wrote: <blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="HOEnZb"><div class="h5">On Mon, May 16, 2016 at 12:54 PM, Rob Clark <<a href="mailto:robdclark@gmail.com">robdclark@gmail.com</a>> wrote: > From: Rob Clark <<a href="mailto:robclark@freedesktop.org">robclark@freedesktop.org</a>> > > Log all the errors, and at the end dump the shader w/ error annotations > to make it easier to see where the problems are. > > Signed-off-by: Rob Clark <<a href="mailto:robclark@freedesktop.org">robclark@freedesktop.org</a>> > Reviewed-by: Eduardo Lima Mitev <<a href="mailto:elima@igalia.com">elima@igalia.com</a>> > --- > src/compiler/nir/nir_validate.c | 60 ++++++++++++++++++++++++++++++++++++++++- > 1 file changed, 59 insertions(+), 1 deletion(-) > > diff --git a/src/compiler/nir/nir_validate.c b/src/compiler/nir/nir_validate.c > index 4e786d4..3e650df 100644 > --- a/src/compiler/nir/nir_validate.c > +++ b/src/compiler/nir/nir_validate.c > @@ -69,6 +69,9 @@ typedef struct { > /* the current instruction being validated */ > nir_instr *instr; > > + /* the current variable being validated */ > + nir_variable *var; > + > /* the current basic block being validated */ > nir_block *block; > > @@ -95,9 +98,30 @@ typedef struct { > > /* map of local variable -> function implementation where it is defined */ > struct hash_table *var_defs; > + > + /* map of instruction/var/etc to failed assert string */ > + struct hash_table *errors; > } validate_state; > > -#define validate_assert(state, cond) assert(cond) > +static void > +log_error(validate_state *state, const char *failed) > +{ > + const void *obj; > + > + if (state->instr) > + obj = state->instr; > + else if (state->var) > + obj = state->var; > + else > + obj = failed; > + > + _mesa_hash_table_insert(state->errors, obj, (void *)failed); > +} > + > +#define validate_assert(state, cond) do { \ > + if (!(cond)) \ > + log_error(state, "error: "#cond); \ </div></div>Maybe add the line number to make the assertion easier to find? Other than that the series gets my r-b. <div class="HOEnZb"><div class="h5"> > + } while (0) > > static void validate_src(nir_src *src, validate_state *state); > > @@ -903,6 +927,8 @@ postvalidate_reg_decl(nir_register *reg, validate_state *state) > static void > validate_var_decl(nir_variable *var, bool is_global, validate_state *state) > { > + state->var = var; > + > validate_assert(state, is_global == nir_variable_is_global(var)); > > /* Must have exactly one mode set */ > @@ -916,6 +942,8 @@ validate_var_decl(nir_variable *var, bool is_global, validate_state *state) > if (!is_global) { > _mesa_hash_table_insert(state->var_defs, var, state->impl); > } > + > + state->var = NULL; > } > > static bool > @@ -1044,7 +1072,12 @@ init_validate_state(validate_state *state) > state->regs_found = NULL; > state->var_defs = _mesa_hash_table_create(NULL, _mesa_hash_pointer, > _mesa_key_pointer_equal); > + state->errors = _mesa_hash_table_create(NULL, _mesa_hash_pointer, > + _mesa_key_pointer_equal); > + > state->loop = NULL; > + state->instr = NULL; > + state->var = NULL; > } > > static void > @@ -1055,6 +1088,28 @@ destroy_validate_state(validate_state *state) > free(state->ssa_defs_found); > free(state->regs_found); > _mesa_hash_table_destroy(state->var_defs, NULL); > + _mesa_hash_table_destroy(state->errors, NULL); > +} > + > +static void > +dump_errors(validate_state *state) > +{ > + struct hash_table *errors = state->errors; > + > + fprintf(stderr, "%d errors:\n", _mesa_hash_table_num_entries(errors)); > + > + nir_print_shader_annotated(state->shader, stderr, errors); > + > + if (_mesa_hash_table_num_entries(errors) > 0) { > + fprintf(stderr, "%d additional errors:\n", > + _mesa_hash_table_num_entries(errors)); > + struct hash_entry *entry; > + hash_table_foreach(errors, entry) { > + fprintf(stderr, "%s\n", (char *)entry->data); > + } > + } > + > + abort(); > } > > void > @@ -1114,6 +1169,9 @@ nir_validate_shader(nir_shader *shader) > postvalidate_reg_decl(reg, &state); > } > > + if (_mesa_hash_table_num_entries(state.errors) > 0) > + dump_errors(&state); > + > destroy_validate_state(&state); > } > > -- > 2.5.5 > </div></div></blockquote></div> </div>