[Mesa-dev] [PATCH] i965: only try print GLSL IR once when using INTEL_DEBUG to dump ir
Alejandro Piñeiro
apinheiro at igalia.com
Fri Nov 4 17:43:26 UTC 2016
Nitpick: I found the commit message hard to understand until I read the
summary.
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?
>
> 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,
More information about the mesa-dev
mailing list