[Mesa-dev] [PATCH 01/53] i965: pass gl_program to the brw_*_debug_recompile() functions

Lionel Landwerlin lionel.g.landwerlin at intel.com
Tue Jan 3 11:33:34 UTC 2017


Looks fine to me :

Reviewed-by: Lionel Landwerlin <lionel.g.landwerlin at intel.com>

On 03/01/17 02:43, Timothy Arceri wrote:
> Rather then passing gl_shader_program.
>
> The only field use was Name which is the same as the Id field in
> gl_program.
>
> For wm and vs we also make the functions static and move them before
> the codegen functions.
>
> This change reduces the codegen functions dependency on gl_shader_program.
> ---
>   src/mesa/drivers/dri/i965/brw_gs.c  |   8 +--
>   src/mesa/drivers/dri/i965/brw_tcs.c |   7 +-
>   src/mesa/drivers/dri/i965/brw_tes.c |   7 +-
>   src/mesa/drivers/dri/i965/brw_vs.c  | 109 ++++++++++++++++---------------
>   src/mesa/drivers/dri/i965/brw_vs.h  |   4 --
>   src/mesa/drivers/dri/i965/brw_wm.c  | 125 ++++++++++++++++++------------------
>   src/mesa/drivers/dri/i965/brw_wm.h  |   3 -
>   7 files changed, 125 insertions(+), 138 deletions(-)
>
> diff --git a/src/mesa/drivers/dri/i965/brw_gs.c b/src/mesa/drivers/dri/i965/brw_gs.c
> index 47b4e5c..76ceb5b 100644
> --- a/src/mesa/drivers/dri/i965/brw_gs.c
> +++ b/src/mesa/drivers/dri/i965/brw_gs.c
> @@ -37,16 +37,14 @@
>   #include "compiler/glsl/ir_uniform.h"
>   
>   static void
> -brw_gs_debug_recompile(struct brw_context *brw,
> -                       struct gl_shader_program *shader_prog,
> +brw_gs_debug_recompile(struct brw_context *brw, struct gl_program *prog,
>                          const struct brw_gs_prog_key *key)
>   {
>      struct brw_cache_item *c = NULL;
>      const struct brw_gs_prog_key *old_key = NULL;
>      bool found = false;
>   
> -   perf_debug("Recompiling geometry shader for program %d\n",
> -              shader_prog->Name);
> +   perf_debug("Recompiling geometry shader for program %d\n", prog->Id);
>   
>      for (unsigned int i = 0; i < brw->cache.size; i++) {
>         for (c = brw->cache.items[i]; c; c = c->next) {
> @@ -164,7 +162,7 @@ brw_codegen_gs_prog(struct brw_context *brw,
>   
>      if (unlikely(brw->perf_debug)) {
>         if (gp->compiled_once) {
> -         brw_gs_debug_recompile(brw, prog, key);
> +         brw_gs_debug_recompile(brw, &gp->program, key);
>         }
>         if (start_busy && !drm_intel_bo_busy(brw->batch.last_bo)) {
>            perf_debug("GS compile took %.03f ms and stalled the GPU\n",
> diff --git a/src/mesa/drivers/dri/i965/brw_tcs.c b/src/mesa/drivers/dri/i965/brw_tcs.c
> index f890ccf..3f8798a 100644
> --- a/src/mesa/drivers/dri/i965/brw_tcs.c
> +++ b/src/mesa/drivers/dri/i965/brw_tcs.c
> @@ -116,8 +116,7 @@ create_passthrough_tcs(void *mem_ctx, const struct brw_compiler *compiler,
>   }
>   
>   static void
> -brw_tcs_debug_recompile(struct brw_context *brw,
> -                       struct gl_shader_program *shader_prog,
> +brw_tcs_debug_recompile(struct brw_context *brw, struct gl_program *prog,
>                          const struct brw_tcs_prog_key *key)
>   {
>      struct brw_cache_item *c = NULL;
> @@ -125,7 +124,7 @@ brw_tcs_debug_recompile(struct brw_context *brw,
>      bool found = false;
>   
>      perf_debug("Recompiling tessellation control shader for program %d\n",
> -              shader_prog->Name);
> +              prog->Id);
>   
>      for (unsigned int i = 0; i < brw->cache.size; i++) {
>         for (c = brw->cache.items[i]; c; c = c->next) {
> @@ -280,7 +279,7 @@ brw_codegen_tcs_prog(struct brw_context *brw,
>      if (unlikely(brw->perf_debug)) {
>         if (tcp) {
>            if (tcp->compiled_once) {
> -            brw_tcs_debug_recompile(brw, shader_prog, key);
> +            brw_tcs_debug_recompile(brw, &tcp->program, key);
>            }
>            tcp->compiled_once = true;
>         }
> diff --git a/src/mesa/drivers/dri/i965/brw_tes.c b/src/mesa/drivers/dri/i965/brw_tes.c
> index 2031366..9adde20 100644
> --- a/src/mesa/drivers/dri/i965/brw_tes.c
> +++ b/src/mesa/drivers/dri/i965/brw_tes.c
> @@ -35,8 +35,7 @@
>   #include "program/prog_parameter.h"
>   
>   static void
> -brw_tes_debug_recompile(struct brw_context *brw,
> -                       struct gl_shader_program *shader_prog,
> +brw_tes_debug_recompile(struct brw_context *brw, struct gl_program *prog,
>                          const struct brw_tes_prog_key *key)
>   {
>      struct brw_cache_item *c = NULL;
> @@ -44,7 +43,7 @@ brw_tes_debug_recompile(struct brw_context *brw,
>      bool found = false;
>   
>      perf_debug("Recompiling tessellation evaluation shader for program %d\n",
> -              shader_prog->Name);
> +              prog->Id);
>   
>      for (unsigned int i = 0; i < brw->cache.size; i++) {
>         for (c = brw->cache.items[i]; c; c = c->next) {
> @@ -194,7 +193,7 @@ brw_codegen_tes_prog(struct brw_context *brw,
>   
>      if (unlikely(brw->perf_debug)) {
>         if (tep->compiled_once) {
> -         brw_tes_debug_recompile(brw, shader_prog, key);
> +         brw_tes_debug_recompile(brw, &tep->program, key);
>         }
>         if (start_busy && !drm_intel_bo_busy(brw->batch.last_bo)) {
>            perf_debug("TES compile took %.03f ms and stalled the GPU\n",
> diff --git a/src/mesa/drivers/dri/i965/brw_vs.c b/src/mesa/drivers/dri/i965/brw_vs.c
> index ede6097..2720f82 100644
> --- a/src/mesa/drivers/dri/i965/brw_vs.c
> +++ b/src/mesa/drivers/dri/i965/brw_vs.c
> @@ -84,6 +84,59 @@ brw_vs_outputs_written(struct brw_context *brw, struct brw_vs_prog_key *key,
>      return outputs_written;
>   }
>   
> +static void
> +brw_vs_debug_recompile(struct brw_context *brw, struct gl_program *prog,
> +                       const struct brw_vs_prog_key *key)
> +{
> +   struct brw_cache_item *c = NULL;
> +   const struct brw_vs_prog_key *old_key = NULL;
> +   bool found = false;
> +
> +   perf_debug("Recompiling vertex shader for program %d\n", prog->Id);
> +
> +   for (unsigned int i = 0; i < brw->cache.size; i++) {
> +      for (c = brw->cache.items[i]; c; c = c->next) {
> +         if (c->cache_id == BRW_CACHE_VS_PROG) {
> +            old_key = c->key;
> +
> +            if (old_key->program_string_id == key->program_string_id)
> +               break;
> +         }
> +      }
> +      if (c)
> +         break;
> +   }
> +
> +   if (!c) {
> +      perf_debug("  Didn't find previous compile in the shader cache for "
> +                 "debug\n");
> +      return;
> +   }
> +
> +   for (unsigned int i = 0; i < VERT_ATTRIB_MAX; i++) {
> +      found |= key_debug(brw, "Vertex attrib w/a flags",
> +                         old_key->gl_attrib_wa_flags[i],
> +                         key->gl_attrib_wa_flags[i]);
> +   }
> +
> +   found |= key_debug(brw, "legacy user clipping",
> +                      old_key->nr_userclip_plane_consts,
> +                      key->nr_userclip_plane_consts);
> +
> +   found |= key_debug(brw, "copy edgeflag",
> +                      old_key->copy_edgeflag, key->copy_edgeflag);
> +   found |= key_debug(brw, "PointCoord replace",
> +                      old_key->point_coord_replace, key->point_coord_replace);
> +   found |= key_debug(brw, "vertex color clamping",
> +                      old_key->clamp_vertex_color, key->clamp_vertex_color);
> +
> +   found |= brw_debug_recompile_sampler_key(brw, &old_key->tex, &key->tex);
> +
> +   if (!found) {
> +      perf_debug("  Something else\n");
> +   }
> +}
> +
>   bool
>   brw_codegen_vs_prog(struct brw_context *brw,
>                       struct gl_shader_program *prog,
> @@ -203,7 +256,7 @@ brw_codegen_vs_prog(struct brw_context *brw,
>   
>      if (unlikely(brw->perf_debug)) {
>         if (vp->compiled_once) {
> -         brw_vs_debug_recompile(brw, prog, key);
> +         brw_vs_debug_recompile(brw, &vp->program, key);
>         }
>         if (start_busy && !drm_intel_bo_busy(brw->batch.last_bo)) {
>            perf_debug("VS compile took %.03f ms and stalled the GPU\n",
> @@ -227,60 +280,6 @@ brw_codegen_vs_prog(struct brw_context *brw,
>      return true;
>   }
>   
> -void
> -brw_vs_debug_recompile(struct brw_context *brw,
> -                       struct gl_shader_program *prog,
> -                       const struct brw_vs_prog_key *key)
> -{
> -   struct brw_cache_item *c = NULL;
> -   const struct brw_vs_prog_key *old_key = NULL;
> -   bool found = false;
> -
> -   perf_debug("Recompiling vertex shader for program %d\n", prog->Name);
> -
> -   for (unsigned int i = 0; i < brw->cache.size; i++) {
> -      for (c = brw->cache.items[i]; c; c = c->next) {
> -         if (c->cache_id == BRW_CACHE_VS_PROG) {
> -            old_key = c->key;
> -
> -            if (old_key->program_string_id == key->program_string_id)
> -               break;
> -         }
> -      }
> -      if (c)
> -         break;
> -   }
> -
> -   if (!c) {
> -      perf_debug("  Didn't find previous compile in the shader cache for "
> -                 "debug\n");
> -      return;
> -   }
> -
> -   for (unsigned int i = 0; i < VERT_ATTRIB_MAX; i++) {
> -      found |= key_debug(brw, "Vertex attrib w/a flags",
> -                         old_key->gl_attrib_wa_flags[i],
> -                         key->gl_attrib_wa_flags[i]);
> -   }
> -
> -   found |= key_debug(brw, "legacy user clipping",
> -                      old_key->nr_userclip_plane_consts,
> -                      key->nr_userclip_plane_consts);
> -
> -   found |= key_debug(brw, "copy edgeflag",
> -                      old_key->copy_edgeflag, key->copy_edgeflag);
> -   found |= key_debug(brw, "PointCoord replace",
> -                      old_key->point_coord_replace, key->point_coord_replace);
> -   found |= key_debug(brw, "vertex color clamping",
> -                      old_key->clamp_vertex_color, key->clamp_vertex_color);
> -
> -   found |= brw_debug_recompile_sampler_key(brw, &old_key->tex, &key->tex);
> -
> -   if (!found) {
> -      perf_debug("  Something else\n");
> -   }
> -}
> -
>   static bool
>   brw_vs_state_dirty(const struct brw_context *brw)
>   {
> diff --git a/src/mesa/drivers/dri/i965/brw_vs.h b/src/mesa/drivers/dri/i965/brw_vs.h
> index 016f2bd..8a5024d 100644
> --- a/src/mesa/drivers/dri/i965/brw_vs.h
> +++ b/src/mesa/drivers/dri/i965/brw_vs.h
> @@ -55,10 +55,6 @@ GLbitfield64
>   brw_vs_outputs_written(struct brw_context *brw, struct brw_vs_prog_key *key,
>                          GLbitfield64 outputs_written);
>   
> -void brw_vs_debug_recompile(struct brw_context *brw,
> -                            struct gl_shader_program *prog,
> -                            const struct brw_vs_prog_key *key);
> -
>   void
>   brw_upload_vs_prog(struct brw_context *brw);
>   
> diff --git a/src/mesa/drivers/dri/i965/brw_wm.c b/src/mesa/drivers/dri/i965/brw_wm.c
> index b0cd163..3a800ad 100644
> --- a/src/mesa/drivers/dri/i965/brw_wm.c
> +++ b/src/mesa/drivers/dri/i965/brw_wm.c
> @@ -68,6 +68,67 @@ assign_fs_binding_table_offsets(const struct gen_device_info *devinfo,
>      }
>   }
>   
> +static void
> +brw_wm_debug_recompile(struct brw_context *brw, struct gl_program *prog,
> +                       const struct brw_wm_prog_key *key)
> +{
> +   struct brw_cache_item *c = NULL;
> +   const struct brw_wm_prog_key *old_key = NULL;
> +   bool found = false;
> +
> +   perf_debug("Recompiling fragment shader for program %d\n", prog->Id);
> +
> +   for (unsigned int i = 0; i < brw->cache.size; i++) {
> +      for (c = brw->cache.items[i]; c; c = c->next) {
> +         if (c->cache_id == BRW_CACHE_FS_PROG) {
> +            old_key = c->key;
> +
> +            if (old_key->program_string_id == key->program_string_id)
> +               break;
> +         }
> +      }
> +      if (c)
> +         break;
> +   }
> +
> +   if (!c) {
> +      perf_debug("  Didn't find previous compile in the shader cache for debug\n");
> +      return;
> +   }
> +
> +   found |= key_debug(brw, "alphatest, computed depth, depth test, or "
> +                      "depth write",
> +                      old_key->iz_lookup, key->iz_lookup);
> +   found |= key_debug(brw, "depth statistics",
> +                      old_key->stats_wm, key->stats_wm);
> +   found |= key_debug(brw, "flat shading",
> +                      old_key->flat_shade, key->flat_shade);
> +   found |= key_debug(brw, "per-sample interpolation",
> +                      old_key->persample_interp, key->persample_interp);
> +   found |= key_debug(brw, "number of color buffers",
> +                      old_key->nr_color_regions, key->nr_color_regions);
> +   found |= key_debug(brw, "MRT alpha test or alpha-to-coverage",
> +                      old_key->replicate_alpha, key->replicate_alpha);
> +   found |= key_debug(brw, "fragment color clamping",
> +                      old_key->clamp_fragment_color, key->clamp_fragment_color);
> +   found |= key_debug(brw, "multisampled FBO",
> +                      old_key->multisample_fbo, key->multisample_fbo);
> +   found |= key_debug(brw, "line smoothing",
> +                      old_key->line_aa, key->line_aa);
> +   found |= key_debug(brw, "input slots valid",
> +                      old_key->input_slots_valid, key->input_slots_valid);
> +   found |= key_debug(brw, "mrt alpha test function",
> +                      old_key->alpha_test_func, key->alpha_test_func);
> +   found |= key_debug(brw, "mrt alpha test reference value",
> +                      old_key->alpha_test_ref, key->alpha_test_ref);
> +
> +   found |= brw_debug_recompile_sampler_key(brw, &old_key->tex, &key->tex);
> +
> +   if (!found) {
> +      perf_debug("  Something else\n");
> +   }
> +}
> +
>   /**
>    * All Mesa program -> GPU code generation goes through this function.
>    * Depending on the instructions used (i.e. flow control instructions)
> @@ -162,7 +223,7 @@ brw_codegen_wm_prog(struct brw_context *brw,
>   
>      if (unlikely(brw->perf_debug)) {
>         if (fp->compiled_once)
> -         brw_wm_debug_recompile(brw, prog, key);
> +         brw_wm_debug_recompile(brw, &fp->program, key);
>         fp->compiled_once = true;
>   
>         if (start_busy && !drm_intel_bo_busy(brw->batch.last_bo)) {
> @@ -233,68 +294,6 @@ brw_debug_recompile_sampler_key(struct brw_context *brw,
>      return found;
>   }
>   
> -void
> -brw_wm_debug_recompile(struct brw_context *brw,
> -                       struct gl_shader_program *prog,
> -                       const struct brw_wm_prog_key *key)
> -{
> -   struct brw_cache_item *c = NULL;
> -   const struct brw_wm_prog_key *old_key = NULL;
> -   bool found = false;
> -
> -   perf_debug("Recompiling fragment shader for program %d\n", prog->Name);
> -
> -   for (unsigned int i = 0; i < brw->cache.size; i++) {
> -      for (c = brw->cache.items[i]; c; c = c->next) {
> -         if (c->cache_id == BRW_CACHE_FS_PROG) {
> -            old_key = c->key;
> -
> -            if (old_key->program_string_id == key->program_string_id)
> -               break;
> -         }
> -      }
> -      if (c)
> -         break;
> -   }
> -
> -   if (!c) {
> -      perf_debug("  Didn't find previous compile in the shader cache for debug\n");
> -      return;
> -   }
> -
> -   found |= key_debug(brw, "alphatest, computed depth, depth test, or "
> -                      "depth write",
> -                      old_key->iz_lookup, key->iz_lookup);
> -   found |= key_debug(brw, "depth statistics",
> -                      old_key->stats_wm, key->stats_wm);
> -   found |= key_debug(brw, "flat shading",
> -                      old_key->flat_shade, key->flat_shade);
> -   found |= key_debug(brw, "per-sample interpolation",
> -                      old_key->persample_interp, key->persample_interp);
> -   found |= key_debug(brw, "number of color buffers",
> -                      old_key->nr_color_regions, key->nr_color_regions);
> -   found |= key_debug(brw, "MRT alpha test or alpha-to-coverage",
> -                      old_key->replicate_alpha, key->replicate_alpha);
> -   found |= key_debug(brw, "fragment color clamping",
> -                      old_key->clamp_fragment_color, key->clamp_fragment_color);
> -   found |= key_debug(brw, "multisampled FBO",
> -                      old_key->multisample_fbo, key->multisample_fbo);
> -   found |= key_debug(brw, "line smoothing",
> -                      old_key->line_aa, key->line_aa);
> -   found |= key_debug(brw, "input slots valid",
> -                      old_key->input_slots_valid, key->input_slots_valid);
> -   found |= key_debug(brw, "mrt alpha test function",
> -                      old_key->alpha_test_func, key->alpha_test_func);
> -   found |= key_debug(brw, "mrt alpha test reference value",
> -                      old_key->alpha_test_ref, key->alpha_test_ref);
> -
> -   found |= brw_debug_recompile_sampler_key(brw, &old_key->tex, &key->tex);
> -
> -   if (!found) {
> -      perf_debug("  Something else\n");
> -   }
> -}
> -
>   static uint8_t
>   gen6_gather_workaround(GLenum internalformat)
>   {
> diff --git a/src/mesa/drivers/dri/i965/brw_wm.h b/src/mesa/drivers/dri/i965/brw_wm.h
> index 135e98f..3a03c16 100644
> --- a/src/mesa/drivers/dri/i965/brw_wm.h
> +++ b/src/mesa/drivers/dri/i965/brw_wm.h
> @@ -65,9 +65,6 @@ bool brw_codegen_wm_prog(struct brw_context *brw,
>                            struct brw_program *fp,
>                            struct brw_wm_prog_key *key,
>                            struct brw_vue_map *vue_map);
> -void brw_wm_debug_recompile(struct brw_context *brw,
> -                            struct gl_shader_program *prog,
> -                            const struct brw_wm_prog_key *key);
>   
>   void
>   brw_upload_wm_prog(struct brw_context *brw);




More information about the mesa-dev mailing list