[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