On Thu, Dec 04, 2014 at 10:02:24PM -0800, Kristian Høgsberg wrote:
> fs_generator no longer knows what stage it's generating code for, so
> we have to set the debug name of the shader from the call site.
>
> Signed-off-by: Kristian Høgsberg <k...@bitplanet.net>
> ---
> src/mesa/drivers/dri/i965/brw_blorp_blit_eu.cpp | 4 +++-
> src/mesa/drivers/dri/i965/brw_fs.cpp | 17 ++++++++++++--
> src/mesa/drivers/dri/i965/brw_fs.h | 7 +++---
> src/mesa/drivers/dri/i965/brw_fs_generator.cpp | 31
> +++++++++++--------------
> 4 files changed, 35 insertions(+), 24 deletions(-)
>
> diff --git a/src/mesa/drivers/dri/i965/brw_blorp_blit_eu.cpp
> b/src/mesa/drivers/dri/i965/brw_blorp_blit_eu.cpp
> index 86ed953..f6d0b68 100644
> --- a/src/mesa/drivers/dri/i965/brw_blorp_blit_eu.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_blorp_blit_eu.cpp
> @@ -31,8 +31,10 @@ brw_blorp_eu_emitter::brw_blorp_eu_emitter(struct
> brw_context *brw,
> : mem_ctx(ralloc_context(NULL)),
> generator(brw, mem_ctx, (void *) rzalloc(mem_ctx, struct
> brw_wm_prog_key),
> (struct brw_stage_prog_data *) rzalloc(mem_ctx, struct
> brw_wm_prog_data),
> - NULL, NULL, false, debug_flag)
> + NULL, NULL, false)
> {
> + if (debug_flag)
> + generator.enable_debug("blorp");
> }
>
> brw_blorp_eu_emitter::~brw_blorp_eu_emitter()
> diff --git a/src/mesa/drivers/dri/i965/brw_fs.cpp
> b/src/mesa/drivers/dri/i965/brw_fs.cpp
> index 6cc508e..8501f72 100644
> --- a/src/mesa/drivers/dri/i965/brw_fs.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_fs.cpp
> @@ -3717,8 +3717,21 @@ brw_wm_fs_emit(struct brw_context *brw,
> prog_data->no_8 = false;
> }
>
> - fs_generator g(brw, mem_ctx, (void *) key, &prog_data->base, prog,
> &fp->Base,
> - v.runtime_check_aads_emit, INTEL_DEBUG & DEBUG_WM);
> + fs_generator g(brw, mem_ctx, (void *) key, &prog_data->base, prog,
> + &fp->Base, v.runtime_check_aads_emit);
> +
> + if (unlikely(INTEL_DEBUG & DEBUG_WM)) {
> + char *name;
> + if (prog)
> + name = ralloc_asprintf(mem_ctx, "%s fragment shader %d",
> + prog->Label ? prog->Label : "unnamed",
> + prog->Name);
> + else
> + name = ralloc_asprintf(mem_ctx, "fragment program %d", fp->Base.Id);
> +
> + g.enable_debug(name);
> + }
> +
> if (simd8_cfg)
> g.generate_code(simd8_cfg, 8);
> if (simd16_cfg)
> diff --git a/src/mesa/drivers/dri/i965/brw_fs.h
> b/src/mesa/drivers/dri/i965/brw_fs.h
> index bba7f96..20c6059 100644
> --- a/src/mesa/drivers/dri/i965/brw_fs.h
> +++ b/src/mesa/drivers/dri/i965/brw_fs.h
> @@ -684,10 +684,10 @@ public:
> struct brw_stage_prog_data *prog_data,
> struct gl_shader_program *shader_prog,
> struct gl_program *fp,
> - bool runtime_check_aads_emit,
> - bool debug_flag);
> + bool runtime_check_aads_emit);
> ~fs_generator();
>
> + void enable_debug(const char *shader_name);
> int generate_code(const cfg_t *cfg, int dispatch_width);
> const unsigned *get_assembly(unsigned int *assembly_size);
>
> @@ -795,7 +795,8 @@ private:
>
> exec_list discard_halt_patches;
> bool runtime_check_aads_emit;
> - const bool debug_flag;
> + bool debug_flag;
> + const char *shader_name;
> void *mem_ctx;
> };
>
> diff --git a/src/mesa/drivers/dri/i965/brw_fs_generator.cpp
> b/src/mesa/drivers/dri/i965/brw_fs_generator.cpp
> index 9d20f40..602595a 100644
> --- a/src/mesa/drivers/dri/i965/brw_fs_generator.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_fs_generator.cpp
> @@ -42,13 +42,12 @@ fs_generator::fs_generator(struct brw_context *brw,
> struct brw_stage_prog_data *prog_data,
> struct gl_shader_program *shader_prog,
> struct gl_program *prog,
> - bool runtime_check_aads_emit,
> - bool debug_flag)
> + bool runtime_check_aads_emit)
>
> : brw(brw), key(key),
> prog_data(prog_data), shader_prog(shader_prog),
> prog(prog), runtime_check_aads_emit(runtime_check_aads_emit),
> - debug_flag(debug_flag), mem_ctx(mem_ctx)
> + debug_flag(false), mem_ctx(mem_ctx)
> {
> ctx = &brw->ctx;
>
> @@ -1512,6 +1511,13 @@ fs_generator::generate_untyped_surface_read(fs_inst
> *inst, struct brw_reg dst,
> brw_mark_surface_used(prog_data, surf_index.dw1.ud);
> }
>
> +void
> +fs_generator::enable_debug(const char *shader_name)
> +{
> + debug_flag = true;
> + this->shader_name = shader_name;
> +}
> +
> int
> fs_generator::generate_code(const cfg_t *cfg, int dispatch_width)
> {
> @@ -2011,21 +2017,10 @@ fs_generator::generate_code(const cfg_t *cfg, int
> dispatch_width)
> int after_size = p->next_insn_offset - start_offset;
>
> if (unlikely(debug_flag)) {
> - if (shader_prog) {
> - fprintf(stderr,
> - "Native code for %s fragment shader %d (SIMD%d
> dispatch):\n",
> - shader_prog->Label ? shader_prog->Label : "unnamed",
> - shader_prog->Name, dispatch_width);
> - } else if (prog) {
> - fprintf(stderr,
> - "Native code for fragment program %d (SIMD%d dispatch):\n",
> - prog->Id, dispatch_width);
> - } else {
> - fprintf(stderr, "Native code for blorp program (SIMD%d
> dispatch):\n",
> - dispatch_width);
> - }
> - fprintf(stderr, "SIMD%d shader: %d instructions. %d loops. Compacted
> %d to %d"
> - " bytes (%.0f%%)\n",
> + fprintf(stderr, "Native code for %s\n"
> + "SIMD%d shader: %d instructions. %d loops. Compacted %d to %d"
> + " bytes (%.0f%%)\n",
> + shader_name,
> dispatch_width, before_size / 16, loop_count, before_size,
> after_size,
> 100.0f * (before_size - after_size) / before_size);
>
I do wonder the original motivation for the debug_flag member. Seems totally
superfluous.
Anyway, lgtm. I was initially going to grumble about adding a new enable_debug()
member, but potentially one could do cool stuff with a disable_debug() at some
point also (perhaps toggling via an API, or something like that).
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
