[Mesa-dev] [PATCH] i965: only try print GLSL IR once when using INTEL_DEBUG to dump ir
Timothy Arceri
timothy.arceri at collabora.com
Fri Nov 4 21:27:47 UTC 2016
On Fri, 2016-11-04 at 18:43 +0100, Alejandro Piñeiro wrote:
> Nitpick: I found the commit message hard to understand until I read
> the
> summary.
Ok I'll duplicate some of the summary in the commit message.
>
> On 03/11/16 11:47, Timothy Arceri wrote:
> >
> > Since we started releasing GLSL IR after linking the only time we
> > can
> > print GLSL IR is during linking. When regenerating variants only
> > NIR
> > will be available.
> > ---
> > src/mesa/drivers/dri/i965/brw_cs.c | 6 ------
> > src/mesa/drivers/dri/i965/brw_gs.c | 3 ---
> > src/mesa/drivers/dri/i965/brw_link.cpp | 10 ++++++++++
> > src/mesa/drivers/dri/i965/brw_program.c | 19 ++++---------------
> > src/mesa/drivers/dri/i965/brw_program.h | 3 +--
> > src/mesa/drivers/dri/i965/brw_tcs.c | 3 ---
> > src/mesa/drivers/dri/i965/brw_tes.c | 3 ---
> > src/mesa/drivers/dri/i965/brw_vs.c | 3 ++-
> > src/mesa/drivers/dri/i965/brw_wm.c | 8 ++++----
> > 9 files changed, 21 insertions(+), 37 deletions(-)
> >
> > diff --git a/src/mesa/drivers/dri/i965/brw_cs.c
> > b/src/mesa/drivers/dri/i965/brw_cs.c
> > index d16fff8..8b13b49 100644
> > --- a/src/mesa/drivers/dri/i965/brw_cs.c
> > +++ b/src/mesa/drivers/dri/i965/brw_cs.c
> > @@ -116,9 +116,6 @@ brw_codegen_cs_prog(struct brw_context *brw,
> > start_time = get_time();
> > }
> >
> > - if (unlikely(INTEL_DEBUG & DEBUG_CS))
> > - brw_dump_ir("compute", prog, &cs->base, &cp->program);
> > -
> > int st_index = -1;
> > if (INTEL_DEBUG & DEBUG_SHADER_TIME)
> > st_index = brw_get_shader_time_index(brw, prog, &cp-
> > >program, ST_CS);
> > @@ -172,9 +169,6 @@ brw_codegen_cs_prog(struct brw_context *brw,
> > prog_data.base.total_scratch,
> > scratch_ids_per_subslice * subslices);
> >
> > - if (unlikely(INTEL_DEBUG & DEBUG_CS))
> > - fprintf(stderr, "\n");
> > -
> > brw_upload_cache(&brw->cache, BRW_CACHE_CS_PROG,
> > key, sizeof(*key),
> > program, program_size,
> > diff --git a/src/mesa/drivers/dri/i965/brw_gs.c
> > b/src/mesa/drivers/dri/i965/brw_gs.c
> > index 0f052d2..77efa63 100644
> > --- a/src/mesa/drivers/dri/i965/brw_gs.c
> > +++ b/src/mesa/drivers/dri/i965/brw_gs.c
> > @@ -143,9 +143,6 @@ brw_codegen_gs_prog(struct brw_context *brw,
> > &prog_data.base.vue_map, outputs_written,
> > prog->SeparateShader);
> >
> > - if (unlikely(INTEL_DEBUG & DEBUG_GS))
> > - brw_dump_ir("geometry", prog, gs, NULL);
> > -
> > int st_index = -1;
> > if (INTEL_DEBUG & DEBUG_SHADER_TIME)
> > st_index = brw_get_shader_time_index(brw, prog, NULL,
> > ST_GS);
> > diff --git a/src/mesa/drivers/dri/i965/brw_link.cpp
> > b/src/mesa/drivers/dri/i965/brw_link.cpp
> > index f75b384..7c8ea3e 100644
> > --- a/src/mesa/drivers/dri/i965/brw_link.cpp
> > +++ b/src/mesa/drivers/dri/i965/brw_link.cpp
> > @@ -266,6 +266,16 @@ brw_link_shader(struct gl_context *ctx, struct
> > gl_shader_program *shProg)
> >
> > brw_add_texrect_params(prog);
> >
> > + bool debug_enabled =
> > + (INTEL_DEBUG & intel_debug_flag_for_shader_stage(shader-
> > >Stage));
> > +
> > + if (debug_enabled && shader->ir) {
> > + fprintf(stderr, "GLSL IR for native %s shader %d:\n",
> > + _mesa_shader_stage_to_string(shader->Stage),
> > shProg->Name);
> > + _mesa_print_ir(stderr, shader->ir, NULL);
> > + fprintf(stderr, "\n\n");
> > + }
> > +
> > prog->nir = brw_create_nir(brw, shProg, prog,
> > (gl_shader_stage) stage,
> > compiler->scalar_stage[stage]);
> >
> > diff --git a/src/mesa/drivers/dri/i965/brw_program.c
> > b/src/mesa/drivers/dri/i965/brw_program.c
> > index 647f138..d45be22 100644
> > --- a/src/mesa/drivers/dri/i965/brw_program.c
> > +++ b/src/mesa/drivers/dri/i965/brw_program.c
> > @@ -638,22 +638,11 @@ brw_stage_prog_data_free(const void *p)
> > }
> >
> > void
> > -brw_dump_ir(const char *stage, struct gl_shader_program
> > *shader_prog,
> > - struct gl_linked_shader *shader, struct gl_program
> > *prog)
> > +brw_dump_arb_asm(const char *stage, struct gl_program *prog)
> > {
> > - if (shader_prog) {
> > - if (shader->ir) {
> > - fprintf(stderr,
> > - "GLSL IR for native %s shader %d:\n",
> > - stage, shader_prog->Name);
> > - _mesa_print_ir(stderr, shader->ir, NULL);
> > - fprintf(stderr, "\n\n");
> > - }
> > - } else {
> > - fprintf(stderr, "ARB_%s_program %d ir for native %s
> > shader\n",
> > - stage, prog->Id, stage);
> > - _mesa_print_program(prog);
> > - }
> > + fprintf(stderr, "ARB_%s_program %d ir for native %s shader\n",
> > + stage, prog->Id, stage);
> > + _mesa_print_program(prog);
> > }
> >
> > void
> > diff --git a/src/mesa/drivers/dri/i965/brw_program.h
> > b/src/mesa/drivers/dri/i965/brw_program.h
> > index eed07a2..43bc625 100644
> > --- a/src/mesa/drivers/dri/i965/brw_program.h
> > +++ b/src/mesa/drivers/dri/i965/brw_program.h
> > @@ -58,8 +58,7 @@ void
> > brw_stage_prog_data_free(const void *prog_data);
> >
> > void
> > -brw_dump_ir(const char *stage, struct gl_shader_program
> > *shader_prog,
> > - struct gl_linked_shader *shader, struct gl_program
> > *prog);
> > +brw_dump_arb_asm(const char *stage, struct gl_program *prog);
> >
> > void brw_upload_tcs_prog(struct brw_context *brw);
> > void brw_tcs_populate_key(struct brw_context *brw,
> > diff --git a/src/mesa/drivers/dri/i965/brw_tcs.c
> > b/src/mesa/drivers/dri/i965/brw_tcs.c
> > index 7beccd7..24cc423 100644
> > --- a/src/mesa/drivers/dri/i965/brw_tcs.c
> > +++ b/src/mesa/drivers/dri/i965/brw_tcs.c
> > @@ -250,9 +250,6 @@ brw_codegen_tcs_prog(struct brw_context *brw,
> > }
> > }
> >
> > - if (unlikely(INTEL_DEBUG & DEBUG_TCS) && tcs)
> > - brw_dump_ir("tessellation control", shader_prog, tcs, NULL);
> > -
> > int st_index = -1;
> > if (unlikely(INTEL_DEBUG & DEBUG_SHADER_TIME))
> > st_index = brw_get_shader_time_index(brw, shader_prog, NULL,
> > ST_TCS);
> > diff --git a/src/mesa/drivers/dri/i965/brw_tes.c
> > b/src/mesa/drivers/dri/i965/brw_tes.c
> > index 6060c76..160f21e 100644
> > --- a/src/mesa/drivers/dri/i965/brw_tes.c
> > +++ b/src/mesa/drivers/dri/i965/brw_tes.c
> > @@ -171,9 +171,6 @@ brw_codegen_tes_prog(struct brw_context *brw,
> > &prog_data.base.base,
> > compiler-
> > >scalar_stage[MESA_SHADER_TESS_EVAL]);
> >
> > - if (unlikely(INTEL_DEBUG & DEBUG_TES))
> > - brw_dump_ir("tessellation evaluation", shader_prog, tes,
> > NULL);
> > -
> > int st_index = -1;
> > if (unlikely(INTEL_DEBUG & DEBUG_SHADER_TIME))
> > st_index = brw_get_shader_time_index(brw, shader_prog, NULL,
> > ST_TES);
> > diff --git a/src/mesa/drivers/dri/i965/brw_vs.c
> > b/src/mesa/drivers/dri/i965/brw_vs.c
> > index 02a88ca..edb577f 100644
> > --- a/src/mesa/drivers/dri/i965/brw_vs.c
> > +++ b/src/mesa/drivers/dri/i965/brw_vs.c
> > @@ -178,7 +178,8 @@ brw_codegen_vs_prog(struct brw_context *brw,
> > }
> >
> > if (unlikely(INTEL_DEBUG & DEBUG_VS)) {
> > - brw_dump_ir("vertex", prog, vs ? &vs->base : NULL, &vp-
> > >program);
> > + if (!prog)
> > + brw_dump_arb_asm("vertex", &vp->program);
>
> I found this check somewhat counterintuitive, in the sense that I
> needed
> to know what the patch is doing and why, and that I needed to see
> what
> brw_dump_ir was doing before this commit. How about adding a small
> comment?
I'll add an extra bit to the commit message about still printing ARB
style programs.
The "if (!prog)" check makes sense when you look at the surrounding
code I'd like to avoid adding a comment to explain it.
Thanks for taking a look.
>
> >
> >
> > fprintf(stderr, "VS Output ");
> > brw_print_vue_map(stderr, &prog_data.base.vue_map);
> > diff --git a/src/mesa/drivers/dri/i965/brw_wm.c
> > b/src/mesa/drivers/dri/i965/brw_wm.c
> > index f433ed6..6babe85 100644
> > --- a/src/mesa/drivers/dri/i965/brw_wm.c
> > +++ b/src/mesa/drivers/dri/i965/brw_wm.c
> > @@ -126,6 +126,9 @@ brw_codegen_wm_prog(struct brw_context *brw,
> > } else {
> > brw_nir_setup_arb_uniforms(fp->program.nir, &fp->program,
> > &prog_data.base);
> > +
> > + if (unlikely(INTEL_DEBUG & DEBUG_WM))
> > + brw_dump_arb_asm("fragment", &fp->program);
> > }
> >
> > if (unlikely(brw->perf_debug)) {
> > @@ -134,9 +137,6 @@ brw_codegen_wm_prog(struct brw_context *brw,
> > start_time = get_time();
> > }
> >
> > - if (unlikely(INTEL_DEBUG & DEBUG_WM))
> > - brw_dump_ir("fragment", prog, fs ? &fs->base : NULL, &fp-
> > >program);
> > -
> > int st_index8 = -1, st_index16 = -1;
> > if (INTEL_DEBUG & DEBUG_SHADER_TIME) {
> > st_index8 = brw_get_shader_time_index(brw, prog, &fp-
> > >program, ST_FS8);
> > @@ -177,7 +177,7 @@ brw_codegen_wm_prog(struct brw_context *brw,
> > prog_data.base.total_scratch,
> > devinfo->max_wm_threads);
> >
> > - if (unlikely(INTEL_DEBUG & DEBUG_WM))
> > + if (unlikely((INTEL_DEBUG & DEBUG_WM) && !prog))
> > fprintf(stderr, "\n");
> >
> > brw_upload_cache(&brw->cache, BRW_CACHE_FS_PROG,
>
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
More information about the mesa-dev
mailing list