[Mesa-dev] [PATCH 20/23] i965: Plumb compiler debug logging through a function pointer in brw_compiler

[Jason Ekstrand]

On Sat, Apr 18, 2015 at 1:55 PM, Kenneth Graunke <kenneth at whitecape.org> wrote:
> On Friday, April 17, 2015 07:12:00 PM Jason Ekstrand wrote:
>> ---
>>  src/mesa/drivers/dri/i965/brw_fs_generator.cpp   | 11 ++++++-----
>>  src/mesa/drivers/dri/i965/brw_shader.cpp         | 13 +++++++++++++
>>  src/mesa/drivers/dri/i965/brw_shader.h           |  2 ++
>>  src/mesa/drivers/dri/i965/brw_vec4_generator.cpp | 11 ++++++-----
>>  4 files changed, 27 insertions(+), 10 deletions(-)
>>
>> diff --git a/src/mesa/drivers/dri/i965/brw_fs_generator.cpp b/src/mesa/drivers/dri/i965/brw_fs_generator.cpp
>> index 35bc241..123bdf7 100644
>> --- a/src/mesa/drivers/dri/i965/brw_fs_generator.cpp
>> +++ b/src/mesa/drivers/dri/i965/brw_fs_generator.cpp
>> @@ -2111,15 +2111,16 @@ fs_generator::generate_code(const cfg_t *cfg, int dispatch_width)
>>        ralloc_free(annotation.ann);
>>     }
>>
>> -   static GLuint msg_id = 0;
>> -   _mesa_gl_debug(&brw->ctx, &msg_id,
>> -                  MESA_DEBUG_SOURCE_SHADER_COMPILER,
>> -                  MESA_DEBUG_TYPE_OTHER,
>> -                  MESA_DEBUG_SEVERITY_NOTIFICATION,
>> +   const int debug_str_size = 160;
>> +   char debug_str[debug_str_size];
>> +   int len;
>> +   len = snprintf(debug_str, debug_str_size,
>>                    "%s SIMD%d shader: %d inst, %d loops, %d:%d spills:fills, "
>>                    "Promoted %u constants, compacted %d to %d bytes.\n",
>>                    stage_abbrev, dispatch_width, before_size / 16, loop_count,
>>                    spill_count, fill_count, promoted_constants, before_size, after_size);
>> +   assert(len < debug_str_size); (void)len;
>> +   brw->intelScreen->compiler->shader_debug_log(debug_str);
>
> I don't like that this requires fixed size buffer logic at every call
> site.  It's kinda gross.
>
> How about making it printf-like instead?  Specifically:
> http://cgit.freedesktop.org/~kwg/mesa/commit/?h=compiler-divorce&id=1a71535d2de01f8a7ad244d39d801d63493ba5e9
> http://cgit.freedesktop.org/~kwg/mesa/commit/?h=compiler-divorce&id=830a25a1f11367e032d8e6a13fa141ff82c06417

Yeah, that's better.  I had thought about it but did the easy thing.
Given that you've got the patches written, I'm 100% with doing that
instead.
--Jason

> (compiler-divorce of my tree has the rebased branch with those patches
> in, if that's useful to you)