[Mesa-dev] [RFC] nir/validate: on failure, dump shader w/ offending line annotated

[Jason Ekstrand]

On Fri, May 13, 2016 at 1:02 PM, Rob Clark <robdclark at gmail.com> wrote:

> From: Rob Clark <robclark at freedesktop.org>
>
> If we assert in nir_validate_shader(), print the shader with the
> offending instruction prefixed with "=>" to make it easier to find what
> part of the shader nir_validate is complaining about.
>
> Macro funny-business in nir_validate() was just to avoid changing a
> bazillion assert() lines to validate_assert() (or similar) for the point
> of an RFC ;-)
>

I love this idea.  I just wish it worked for more than just instructions.
It would also be fantastic if it were somehow able to print more than one
error.  Maybe something where we tie printing and validation together
somehow?  Just a thought.


> Example output: http://hastebin.com/raw/qorirayazu
> ---
>  src/compiler/nir/nir.h          |  1 +
>  src/compiler/nir/nir_print.c    | 14 +++++++++++++-
>  src/compiler/nir/nir_validate.c | 15 +++++++++++++++
>  3 files changed, 29 insertions(+), 1 deletion(-)
>
> diff --git a/src/compiler/nir/nir.h b/src/compiler/nir/nir.h
> index ade584c..6bb9fbe 100644
> --- a/src/compiler/nir/nir.h
> +++ b/src/compiler/nir/nir.h
> @@ -2196,6 +2196,7 @@ unsigned nir_index_instrs(nir_function_impl *impl);
>  void nir_index_blocks(nir_function_impl *impl);
>
>  void nir_print_shader(nir_shader *shader, FILE *fp);
> +void nir_print_shader_err(nir_shader *shader, FILE *fp, nir_instr *instr);
>  void nir_print_instr(const nir_instr *instr, FILE *fp);
>
>  nir_shader *nir_shader_clone(void *mem_ctx, const nir_shader *s);
> diff --git a/src/compiler/nir/nir_print.c b/src/compiler/nir/nir_print.c
> index a36561e..3b25a49 100644
> --- a/src/compiler/nir/nir_print.c
> +++ b/src/compiler/nir/nir_print.c
> @@ -53,6 +53,8 @@ typedef struct {
>
>     /* an index used to make new non-conflicting names */
>     unsigned index;
> +
> +   nir_instr *err_instr;
>  } print_state;
>
>  static void
> @@ -916,6 +918,8 @@ print_block(nir_block *block, print_state *state,
> unsigned tabs)
>     free(preds);
>
>     nir_foreach_instr(instr, block) {
> +      if (instr == state->err_instr)
> +         fprintf(fp, "=>");
>        print_instr(instr, state, tabs);
>        fprintf(fp, "\n");
>     }
> @@ -1090,11 +1094,13 @@ destroy_print_state(print_state *state)
>  }
>
>  void
> -nir_print_shader(nir_shader *shader, FILE *fp)
> +nir_print_shader_err(nir_shader *shader, FILE *fp, nir_instr *instr)
>  {
>     print_state state;
>     init_print_state(&state, shader, fp);
>
> +   state.err_instr = instr;
> +
>     fprintf(fp, "shader: %s\n", gl_shader_stage_name(shader->stage));
>
>     if (shader->info.name)
> @@ -1144,6 +1150,12 @@ nir_print_shader(nir_shader *shader, FILE *fp)
>  }
>
>  void
> +nir_print_shader(nir_shader *shader, FILE *fp)
> +{
> +   nir_print_shader_err(shader, fp, NULL);
> +}
> +
> +void
>  nir_print_instr(const nir_instr *instr, FILE *fp)
>  {
>     print_state state = {
> diff --git a/src/compiler/nir/nir_validate.c
> b/src/compiler/nir/nir_validate.c
> index 84334d4..b47087f 100644
> --- a/src/compiler/nir/nir_validate.c
> +++ b/src/compiler/nir/nir_validate.c
> @@ -97,6 +97,21 @@ typedef struct {
>     struct hash_table *var_defs;
>  } validate_state;
>
> +
> +
> +static void
> +dump_assert(validate_state *state, const char *failed)
> +{
> +   fprintf(stderr, "validate failed: %s\n", failed);
> +   if (state->instr)
> +      nir_print_shader_err(state->shader, stderr, state->instr);
> +}
> +
> +#define __assert assert
> +#undef assert
> +#define assert(x) do { if (!(x)) { dump_assert(state, #x);
> __assert_fail(#x, __FILE__, __LINE__, __func__); } } while (0)
> +
> +
>  static void validate_src(nir_src *src, validate_state *state);
>
>  static void
> --
> 2.5.5
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20160513/bdd2b1dc/attachment-0001.html>