[Mesa-dev] [PATCH] i965: only try print GLSL IR once when using INTEL_DEBUG to dump ir
Alejandro Piñeiro
apinheiro at igalia.com
Sat Nov 5 08:44:11 UTC 2016
On 04/11/16 22:27, Timothy Arceri wrote:
> 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.
Ok, thanks.
>> 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.
Ok, makes sense.
>
> Thanks for taking a look.
Thanks for the patch.
>
>>>
>>> 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