Discussion:
[Mesa-dev] [PATCH 01/11] mesa: properly report the length of truncated log messages
Mark Janes
2018-12-07 00:35:42 UTC
Permalink
_mesa_log_msg must provide the length of the string passed into the
KHR_debug api. When the string formatted by _mesa_gl_vdebugf exceeds
MAX_DEBUG_MESSAGE_LENGTH, the length is incorrectly set to the number
of characters that would have been written if enough space had been
available.

Fixes: 30256805784450b8bb9d4dabfb56226271ca9d24
("mesa: Add support for GL_ARB_debug_output with dynamic ID allocation.")
---
src/mesa/main/errors.c | 3 +++
1 file changed, 3 insertions(+)

diff --git a/src/mesa/main/errors.c b/src/mesa/main/errors.c
index a9687913627..30560ba047e 100644
--- a/src/mesa/main/errors.c
+++ b/src/mesa/main/errors.c
@@ -231,6 +231,9 @@ _mesa_gl_vdebug(struct gl_context *ctx,
_mesa_debug_get_id(id);

len = _mesa_vsnprintf(s, MAX_DEBUG_MESSAGE_LENGTH, fmtString, args);
+ if (len >= MAX_DEBUG_MESSAGE_LENGTH)
+ /* message was truncated */
+ len = MAX_DEBUG_MESSAGE_LENGTH - 1;

_mesa_log_msg(ctx, source, type, *id, severity, len, s);
}
--
2.19.2
Mark Janes
2018-12-07 00:35:43 UTC
Permalink
In preparation for the definition of a function to log a formatted
string.
---
src/mesa/drivers/dri/i915/intel_context.h | 18 +++++------
src/mesa/drivers/dri/i915/intel_fbo.c | 10 +++---
src/mesa/drivers/dri/i965/brw_context.h | 18 +++++------
src/mesa/drivers/dri/i965/genX_state_upload.c | 14 ++++----
src/mesa/drivers/dri/i965/intel_fbo.c | 10 +++---
src/mesa/drivers/dri/i965/intel_screen.c | 16 +++++-----
src/mesa/main/bufferobj.c | 10 +++---
src/mesa/main/errors.c | 28 ++++++++--------
src/mesa/main/errors.h | 32 +++++++++----------
src/mesa/main/fbobject.c | 10 +++---
src/mesa/main/pipelineobj.c | 16 +++++-----
src/mesa/state_tracker/st_debug.c | 2 +-
12 files changed, 92 insertions(+), 92 deletions(-)

diff --git a/src/mesa/drivers/dri/i915/intel_context.h b/src/mesa/drivers/dri/i915/intel_context.h
index b79288d29bc..2ed1a1444e5 100644
--- a/src/mesa/drivers/dri/i915/intel_context.h
+++ b/src/mesa/drivers/dri/i915/intel_context.h
@@ -304,11 +304,11 @@ extern int INTEL_DEBUG;
if (unlikely(INTEL_DEBUG & DEBUG_PERF)) \
dbg_printf(__VA_ARGS__); \
if (intel->perf_debug) \
- _mesa_gl_debug(&intel->ctx, &msg_id, \
- MESA_DEBUG_SOURCE_API, \
- MESA_DEBUG_TYPE_PERFORMANCE, \
- MESA_DEBUG_SEVERITY_MEDIUM, \
- __VA_ARGS__); \
+ _mesa_gl_debugf(&intel->ctx, &msg_id, \
+ MESA_DEBUG_SOURCE_API, \
+ MESA_DEBUG_TYPE_PERFORMANCE, \
+ MESA_DEBUG_SEVERITY_MEDIUM, \
+ __VA_ARGS__); \
} while(0)

#define WARN_ONCE(cond, fmt...) do { \
@@ -320,10 +320,10 @@ extern int INTEL_DEBUG;
fprintf(stderr, fmt); \
_warned = true; \
\
- _mesa_gl_debug(ctx, &msg_id, \
- MESA_DEBUG_SOURCE_API, \
- MESA_DEBUG_TYPE_OTHER, \
- MESA_DEBUG_SEVERITY_HIGH, fmt); \
+ _mesa_gl_debugf(ctx, &msg_id, \
+ MESA_DEBUG_SOURCE_API, \
+ MESA_DEBUG_TYPE_OTHER, \
+ MESA_DEBUG_SEVERITY_HIGH, fmt); \
} \
} \
} while (0)
diff --git a/src/mesa/drivers/dri/i915/intel_fbo.c b/src/mesa/drivers/dri/i915/intel_fbo.c
index 78e2c1e6614..880fd7cfba1 100644
--- a/src/mesa/drivers/dri/i915/intel_fbo.c
+++ b/src/mesa/drivers/dri/i915/intel_fbo.c
@@ -536,11 +536,11 @@ intel_finish_render_texture(struct gl_context * ctx, struct gl_renderbuffer *rb)
#define fbo_incomplete(fb, ...) do { \
static GLuint msg_id = 0; \
if (unlikely(ctx->Const.ContextFlags & GL_CONTEXT_FLAG_DEBUG_BIT)) { \
- _mesa_gl_debug(ctx, &msg_id, \
- MESA_DEBUG_SOURCE_API, \
- MESA_DEBUG_TYPE_OTHER, \
- MESA_DEBUG_SEVERITY_MEDIUM, \
- __VA_ARGS__); \
+ _mesa_gl_debugf(ctx, &msg_id, \
+ MESA_DEBUG_SOURCE_API, \
+ MESA_DEBUG_TYPE_OTHER, \
+ MESA_DEBUG_SEVERITY_MEDIUM, \
+ __VA_ARGS__); \
} \
DBG(__VA_ARGS__); \
fb->_Status = GL_FRAMEBUFFER_UNSUPPORTED; \
diff --git a/src/mesa/drivers/dri/i965/brw_context.h b/src/mesa/drivers/dri/i965/brw_context.h
index e5d57dd8df1..7374eef9fb5 100644
--- a/src/mesa/drivers/dri/i965/brw_context.h
+++ b/src/mesa/drivers/dri/i965/brw_context.h
@@ -388,11 +388,11 @@ struct brw_cache {
if (unlikely(INTEL_DEBUG & DEBUG_PERF)) \
dbg_printf(__VA_ARGS__); \
if (brw->perf_debug) \
- _mesa_gl_debug(&brw->ctx, &msg_id, \
- MESA_DEBUG_SOURCE_API, \
- MESA_DEBUG_TYPE_PERFORMANCE, \
- MESA_DEBUG_SEVERITY_MEDIUM, \
- __VA_ARGS__); \
+ _mesa_gl_debugf(&brw->ctx, &msg_id, \
+ MESA_DEBUG_SOURCE_API, \
+ MESA_DEBUG_TYPE_PERFORMANCE, \
+ MESA_DEBUG_SEVERITY_MEDIUM, \
+ __VA_ARGS__); \
} while(0)

#define WARN_ONCE(cond, fmt...) do { \
@@ -404,10 +404,10 @@ struct brw_cache {
fprintf(stderr, fmt); \
_warned = true; \
\
- _mesa_gl_debug(ctx, &msg_id, \
- MESA_DEBUG_SOURCE_API, \
- MESA_DEBUG_TYPE_OTHER, \
- MESA_DEBUG_SEVERITY_HIGH, fmt); \
+ _mesa_gl_debugf(ctx, &msg_id, \
+ MESA_DEBUG_SOURCE_API, \
+ MESA_DEBUG_TYPE_OTHER, \
+ MESA_DEBUG_SEVERITY_HIGH, fmt); \
} \
} \
} while (0)
diff --git a/src/mesa/drivers/dri/i965/genX_state_upload.c b/src/mesa/drivers/dri/i965/genX_state_upload.c
index 8e3fcbf12ec..6ea7b3758e1 100644
--- a/src/mesa/drivers/dri/i965/genX_state_upload.c
+++ b/src/mesa/drivers/dri/i965/genX_state_upload.c
@@ -3297,13 +3297,13 @@ genX(upload_push_constant_packets)(struct brw_context *brw)

if (binding->BufferObject == ctx->Shared->NullBufferObj) {
static unsigned msg_id = 0;
- _mesa_gl_debug(ctx, &msg_id, MESA_DEBUG_SOURCE_API,
- MESA_DEBUG_TYPE_UNDEFINED,
- MESA_DEBUG_SEVERITY_HIGH,
- "UBO %d unbound, %s shader uniform data "
- "will be undefined.",
- range->block,
- _mesa_shader_stage_to_string(stage));
+ _mesa_gl_debugf(ctx, &msg_id, MESA_DEBUG_SOURCE_API,
+ MESA_DEBUG_TYPE_UNDEFINED,
+ MESA_DEBUG_SEVERITY_HIGH,
+ "UBO %d unbound, %s shader uniform data "
+ "will be undefined.",
+ range->block,
+ _mesa_shader_stage_to_string(stage));
continue;
}

diff --git a/src/mesa/drivers/dri/i965/intel_fbo.c b/src/mesa/drivers/dri/i965/intel_fbo.c
index 5bcd846a1bc..b3ba09f2174 100644
--- a/src/mesa/drivers/dri/i965/intel_fbo.c
+++ b/src/mesa/drivers/dri/i965/intel_fbo.c
@@ -632,11 +632,11 @@ intel_render_texture(struct gl_context * ctx,
#define fbo_incomplete(fb, error_id, ...) do { \
static GLuint msg_id = 0; \
if (unlikely(ctx->Const.ContextFlags & GL_CONTEXT_FLAG_DEBUG_BIT)) { \
- _mesa_gl_debug(ctx, &msg_id, \
- MESA_DEBUG_SOURCE_API, \
- MESA_DEBUG_TYPE_OTHER, \
- MESA_DEBUG_SEVERITY_MEDIUM, \
- __VA_ARGS__); \
+ _mesa_gl_debugf(ctx, &msg_id, \
+ MESA_DEBUG_SOURCE_API, \
+ MESA_DEBUG_TYPE_OTHER, \
+ MESA_DEBUG_SEVERITY_MEDIUM, \
+ __VA_ARGS__); \
} \
DBG(__VA_ARGS__); \
fb->_Status = error_id; \
diff --git a/src/mesa/drivers/dri/i965/intel_screen.c b/src/mesa/drivers/dri/i965/intel_screen.c
index 8838f977bb6..c0201357869 100644
--- a/src/mesa/drivers/dri/i965/intel_screen.c
+++ b/src/mesa/drivers/dri/i965/intel_screen.c
@@ -2396,10 +2396,10 @@ shader_debug_log_mesa(void *data, const char *fmt, ...)

va_start(args, fmt);
GLuint msg_id = 0;
- _mesa_gl_vdebug(&brw->ctx, &msg_id,
- MESA_DEBUG_SOURCE_SHADER_COMPILER,
- MESA_DEBUG_TYPE_OTHER,
- MESA_DEBUG_SEVERITY_NOTIFICATION, fmt, args);
+ _mesa_gl_vdebugf(&brw->ctx, &msg_id,
+ MESA_DEBUG_SOURCE_SHADER_COMPILER,
+ MESA_DEBUG_TYPE_OTHER,
+ MESA_DEBUG_SEVERITY_NOTIFICATION, fmt, args);
va_end(args);
}

@@ -2420,10 +2420,10 @@ shader_perf_log_mesa(void *data, const char *fmt, ...)

if (brw->perf_debug) {
GLuint msg_id = 0;
- _mesa_gl_vdebug(&brw->ctx, &msg_id,
- MESA_DEBUG_SOURCE_SHADER_COMPILER,
- MESA_DEBUG_TYPE_PERFORMANCE,
- MESA_DEBUG_SEVERITY_MEDIUM, fmt, args);
+ _mesa_gl_vdebugf(&brw->ctx, &msg_id,
+ MESA_DEBUG_SOURCE_SHADER_COMPILER,
+ MESA_DEBUG_TYPE_PERFORMANCE,
+ MESA_DEBUG_SEVERITY_MEDIUM, fmt, args);
}
va_end(args);
}
diff --git a/src/mesa/main/bufferobj.c b/src/mesa/main/bufferobj.c
index 534326858bb..c22f1986e7d 100644
--- a/src/mesa/main/bufferobj.c
+++ b/src/mesa/main/bufferobj.c
@@ -73,11 +73,11 @@ buffer_usage_warning(struct gl_context *ctx, GLuint *id, const char *fmt, ...)
va_list args;

va_start(args, fmt);
- _mesa_gl_vdebug(ctx, id,
- MESA_DEBUG_SOURCE_API,
- MESA_DEBUG_TYPE_PERFORMANCE,
- MESA_DEBUG_SEVERITY_MEDIUM,
- fmt, args);
+ _mesa_gl_vdebugf(ctx, id,
+ MESA_DEBUG_SOURCE_API,
+ MESA_DEBUG_TYPE_PERFORMANCE,
+ MESA_DEBUG_SEVERITY_MEDIUM,
+ fmt, args);
va_end(args);
}

diff --git a/src/mesa/main/errors.c b/src/mesa/main/errors.c
index 30560ba047e..fad8cb59cae 100644
--- a/src/mesa/main/errors.c
+++ b/src/mesa/main/errors.c
@@ -217,13 +217,13 @@ should_output(struct gl_context *ctx, GLenum error, const char *fmtString)


void
-_mesa_gl_vdebug(struct gl_context *ctx,
- GLuint *id,
- enum mesa_debug_source source,
- enum mesa_debug_type type,
- enum mesa_debug_severity severity,
- const char *fmtString,
- va_list args)
+_mesa_gl_vdebugf(struct gl_context *ctx,
+ GLuint *id,
+ enum mesa_debug_source source,
+ enum mesa_debug_type type,
+ enum mesa_debug_severity severity,
+ const char *fmtString,
+ va_list args)
{
char s[MAX_DEBUG_MESSAGE_LENGTH];
int len;
@@ -240,16 +240,16 @@ _mesa_gl_vdebug(struct gl_context *ctx,


void
-_mesa_gl_debug(struct gl_context *ctx,
- GLuint *id,
- enum mesa_debug_source source,
- enum mesa_debug_type type,
- enum mesa_debug_severity severity,
- const char *fmtString, ...)
+_mesa_gl_debugf(struct gl_context *ctx,
+ GLuint *id,
+ enum mesa_debug_source source,
+ enum mesa_debug_type type,
+ enum mesa_debug_severity severity,
+ const char *fmtString, ...)
{
va_list args;
va_start(args, fmtString);
- _mesa_gl_vdebug(ctx, id, source, type, severity, fmtString, args);
+ _mesa_gl_vdebugf(ctx, id, source, type, severity, fmtString, args);
va_end(args);
}

diff --git a/src/mesa/main/errors.h b/src/mesa/main/errors.h
index 5911da2956f..3e2d56e7741 100644
--- a/src/mesa/main/errors.h
+++ b/src/mesa/main/errors.h
@@ -74,30 +74,30 @@ _mesa_shader_debug(struct gl_context *ctx, GLenum type, GLuint *id,
const char *msg);

extern void
-_mesa_gl_vdebug(struct gl_context *ctx,
+_mesa_gl_vdebugf(struct gl_context *ctx,
+ GLuint *id,
+ enum mesa_debug_source source,
+ enum mesa_debug_type type,
+ enum mesa_debug_severity severity,
+ const char *fmtString,
+ va_list args);
+
+extern void
+_mesa_gl_debugf(struct gl_context *ctx,
GLuint *id,
enum mesa_debug_source source,
enum mesa_debug_type type,
enum mesa_debug_severity severity,
- const char *fmtString,
- va_list args);
-
-extern void
-_mesa_gl_debug(struct gl_context *ctx,
- GLuint *id,
- enum mesa_debug_source source,
- enum mesa_debug_type type,
- enum mesa_debug_severity severity,
- const char *fmtString, ...) PRINTFLIKE(6, 7);
+ const char *fmtString, ...) PRINTFLIKE(6, 7);

#define _mesa_perf_debug(ctx, sev, ...) do { \
static GLuint msg_id = 0; \
if (unlikely(ctx->Const.ContextFlags & GL_CONTEXT_FLAG_DEBUG_BIT)) { \
- _mesa_gl_debug(ctx, &msg_id, \
- MESA_DEBUG_SOURCE_API, \
- MESA_DEBUG_TYPE_PERFORMANCE, \
- sev, \
- __VA_ARGS__); \
+ _mesa_gl_debugf(ctx, &msg_id, \
+ MESA_DEBUG_SOURCE_API, \
+ MESA_DEBUG_TYPE_PERFORMANCE, \
+ sev, \
+ __VA_ARGS__); \
} \
} while (0)

diff --git a/src/mesa/main/fbobject.c b/src/mesa/main/fbobject.c
index 68e0daf3423..a5b829c9ad2 100644
--- a/src/mesa/main/fbobject.c
+++ b/src/mesa/main/fbobject.c
@@ -674,11 +674,11 @@ fbo_incomplete(struct gl_context *ctx, const char *msg, int index)
{
static GLuint msg_id;

- _mesa_gl_debug(ctx, &msg_id,
- MESA_DEBUG_SOURCE_API,
- MESA_DEBUG_TYPE_OTHER,
- MESA_DEBUG_SEVERITY_MEDIUM,
- "FBO incomplete: %s [%d]\n", msg, index);
+ _mesa_gl_debugf(ctx, &msg_id,
+ MESA_DEBUG_SOURCE_API,
+ MESA_DEBUG_TYPE_OTHER,
+ MESA_DEBUG_SEVERITY_MEDIUM,
+ "FBO incomplete: %s [%d]\n", msg, index);

if (MESA_DEBUG_FLAGS & DEBUG_INCOMPLETE_FBO) {
_mesa_debug(NULL, "FBO Incomplete: %s [%d]\n", msg, index);
diff --git a/src/mesa/main/pipelineobj.c b/src/mesa/main/pipelineobj.c
index b4ca14d7257..1fe1205fb52 100644
--- a/src/mesa/main/pipelineobj.c
+++ b/src/mesa/main/pipelineobj.c
@@ -1019,14 +1019,14 @@ _mesa_validate_program_pipeline(struct gl_context* ctx,

static GLuint msg_id = 0;

- _mesa_gl_debug(ctx, &msg_id,
- MESA_DEBUG_SOURCE_API,
- MESA_DEBUG_TYPE_PORTABILITY,
- MESA_DEBUG_SEVERITY_MEDIUM,
- "glValidateProgramPipeline: pipeline %u does not meet "
- "strict OpenGL ES 3.1 requirements and may not be "
- "portable across desktop hardware\n",
- pipe->Name);
+ _mesa_gl_debugf(ctx, &msg_id,
+ MESA_DEBUG_SOURCE_API,
+ MESA_DEBUG_TYPE_PORTABILITY,
+ MESA_DEBUG_SEVERITY_MEDIUM,
+ "glValidateProgramPipeline: pipeline %u does not meet "
+ "strict OpenGL ES 3.1 requirements and may not be "
+ "portable across desktop hardware\n",
+ pipe->Name);
}

pipe->Validated = GL_TRUE;
diff --git a/src/mesa/state_tracker/st_debug.c b/src/mesa/state_tracker/st_debug.c
index d6cb5cd57d8..27d50a17e5e 100644
--- a/src/mesa/state_tracker/st_debug.c
+++ b/src/mesa/state_tracker/st_debug.c
@@ -161,7 +161,7 @@ st_debug_message(void *data,
default:
unreachable("invalid debug type");
}
- _mesa_gl_vdebug(st->ctx, id, source, type, severity, fmt, args);
+ _mesa_gl_vdebugf(st->ctx, id, source, type, severity, fmt, args);
}

void
--
2.19.2
Mark Janes
2018-12-07 00:35:52 UTC
Permalink
---
src/mesa/drivers/dri/i965/brw_disk_cache.c | 23 ++++++++++++++++++----
1 file changed, 19 insertions(+), 4 deletions(-)

diff --git a/src/mesa/drivers/dri/i965/brw_disk_cache.c b/src/mesa/drivers/dri/i965/brw_disk_cache.c
index 65fcab24b7f..0acbef4f20e 100644
--- a/src/mesa/drivers/dri/i965/brw_disk_cache.c
+++ b/src/mesa/drivers/dri/i965/brw_disk_cache.c
@@ -199,16 +199,31 @@ read_and_upload(struct brw_context *brw, struct disk_cache *cache,
brw_alloc_stage_scratch(brw, stage_state, prog_data->total_scratch);

if (unlikely(debug_enabled_for_stage(stage))) {
- fprintf(stderr, "NIR for %s program %d loaded from disk shader cache:\n",
+ char *buf;
+ size_t buf_size;
+ FILE * log_fp = open_memstream(&buf, &buf_size);
+ fprintf(log_fp, "NIR for %s program %d loaded from disk shader cache:\n",
_mesa_shader_stage_to_abbrev(stage), brw_program(prog)->id);
brw_program_deserialize_driver_blob(&brw->ctx, prog, stage);
nir_shader *nir = prog->nir;
- nir_print_shader(nir, stderr);
- fprintf(stderr, "Native code for %s %s shader %s from disk cache:\n",
+ nir_print_shader(nir, log_fp);
+ fclose(log_fp);
+ static GLuint nir_msg_id = 0;
+ shader_debug_log_mesa(brw, &nir_msg_id, buf);
+ fputs(buf, stderr);
+ free(buf);
+
+ log_fp = open_memstream(&buf, &buf_size);
+ fprintf(log_fp, "Native code for %s %s shader %s from disk cache:\n",
nir->info.label ? nir->info.label : "unnamed",
_mesa_shader_stage_to_string(nir->info.stage), nir->info.name);
brw_disassemble(&brw->screen->devinfo, program, 0,
- prog_data->program_size, stderr);
+ prog_data->program_size, log_fp);
+ fclose(log_fp);
+ static GLuint native_msg_id = 0;
+ shader_debug_log_mesa(brw, &native_msg_id, buf);
+ fputs(buf, stderr);
+ free(buf);
}

brw_upload_cache(&brw->cache, cache_id, &prog_key, brw_prog_key_size(stage),
--
2.19.2
Mark Janes
2018-12-07 00:35:48 UTC
Permalink
---
src/intel/compiler/brw_compiler.h | 2 +-
src/intel/compiler/brw_fs_generator.cpp | 2 +-
src/intel/compiler/brw_vec4_generator.cpp | 3 +--
src/intel/vulkan/anv_device.c | 2 +-
src/mesa/drivers/dri/i965/intel_screen.c | 19 ++++++++++---------
5 files changed, 14 insertions(+), 14 deletions(-)

diff --git a/src/intel/compiler/brw_compiler.h b/src/intel/compiler/brw_compiler.h
index ca9d41ca7a8..816b96b24e6 100644
--- a/src/intel/compiler/brw_compiler.h
+++ b/src/intel/compiler/brw_compiler.h
@@ -89,7 +89,7 @@ struct brw_compiler {
int aligned_pairs_class;
} fs_reg_sets[3];

- void (*shader_debug_log)(void *, GLuint *msg_id, const char *str, ...) PRINTFLIKE(3, 4);
+ void (*shader_debug_log)(void *, GLuint *msg_id, const char *buf);
void (*shader_perf_log)(void *, const char *str, ...) PRINTFLIKE(2, 3);

bool scalar_stage[MESA_SHADER_STAGES];
diff --git a/src/intel/compiler/brw_fs_generator.cpp b/src/intel/compiler/brw_fs_generator.cpp
index 6b80f8433b2..10cc6b2ff15 100644
--- a/src/intel/compiler/brw_fs_generator.cpp
+++ b/src/intel/compiler/brw_fs_generator.cpp
@@ -2487,7 +2487,7 @@ fs_generator::generate_code(const cfg_t *cfg, int dispatch_width)
assert(validated);

static GLuint msg_id = 0;
- compiler->shader_debug_log(log_data, &msg_id, "%s", buf);
+ compiler->shader_debug_log(log_data, &msg_id, buf);
free(buf);
return start_offset;
}
diff --git a/src/intel/compiler/brw_vec4_generator.cpp b/src/intel/compiler/brw_vec4_generator.cpp
index f56ec6aa9a1..0ed1c8d99fa 100644
--- a/src/intel/compiler/brw_vec4_generator.cpp
+++ b/src/intel/compiler/brw_vec4_generator.cpp
@@ -2220,8 +2220,7 @@ generate_code(struct brw_codegen *p,

fclose(log_fp);
static GLuint msg_id = 0;
- compiler->shader_debug_log(log_data, &msg_id,
- "%s", buf);
+ compiler->shader_debug_log(log_data, &msg_id, buf);
free(buf);
}

diff --git a/src/intel/vulkan/anv_device.c b/src/intel/vulkan/anv_device.c
index e60f7327ec1..f8ecf2f6991 100644
--- a/src/intel/vulkan/anv_device.c
+++ b/src/intel/vulkan/anv_device.c
@@ -45,7 +45,7 @@
#include "genxml/gen7_pack.h"

static void
-compiler_debug_log(void *data, GLuint *id, const char *fmt, ...)
+compiler_debug_log(void *data, GLuint *id, const char *buf)
{ }

static void
diff --git a/src/mesa/drivers/dri/i965/intel_screen.c b/src/mesa/drivers/dri/i965/intel_screen.c
index 6b54767f8dd..ed0eaa783db 100644
--- a/src/mesa/drivers/dri/i965/intel_screen.c
+++ b/src/mesa/drivers/dri/i965/intel_screen.c
@@ -2389,17 +2389,18 @@ intel_device_get_revision(int fd)
}

static void
-shader_debug_log_mesa(void *data, GLuint *msg_id, const char *fmt, ...)
+shader_debug_log_mesa(void *data, GLuint *msg_id, const char *buf)
{
struct brw_context *brw = (struct brw_context *)data;
- va_list args;
-
- va_start(args, fmt);
- _mesa_gl_vdebugf(&brw->ctx, msg_id,
- MESA_DEBUG_SOURCE_SHADER_COMPILER,
- MESA_DEBUG_TYPE_OTHER,
- MESA_DEBUG_SEVERITY_NOTIFICATION, fmt, args);
- va_end(args);
+ const size_t len = strlen(buf);
+ const char *buf_ptr = buf;
+ while (buf_ptr < buf + len) {
+ buf_ptr += _mesa_gl_debug(&brw->ctx, msg_id,
+ MESA_DEBUG_SOURCE_SHADER_COMPILER,
+ MESA_DEBUG_TYPE_OTHER,
+ MESA_DEBUG_SEVERITY_NOTIFICATION,
+ buf_ptr);
+ }
}

static void
--
2.19.2
Mark Janes
2018-12-07 00:35:45 UTC
Permalink
---
src/intel/compiler/brw_disasm_info.c | 28 +++++++++++------------
src/intel/compiler/brw_disasm_info.h | 2 +-
src/intel/compiler/brw_fs_generator.cpp | 2 +-
src/intel/compiler/brw_vec4_generator.cpp | 2 +-
src/intel/compiler/test_eu_validate.cpp | 2 +-
5 files changed, 18 insertions(+), 18 deletions(-)

diff --git a/src/intel/compiler/brw_disasm_info.c b/src/intel/compiler/brw_disasm_info.c
index 7a9a55d83ef..21a1170b0a2 100644
--- a/src/intel/compiler/brw_disasm_info.c
+++ b/src/intel/compiler/brw_disasm_info.c
@@ -31,7 +31,7 @@ __attribute__((weak)) void nir_print_instr(UNUSED const nir_instr *instr,
UNUSED FILE *fp) {}

void
-dump_assembly(void *assembly, struct disasm_info *disasm)
+dump_assembly(FILE *fp, void *assembly, struct disasm_info *disasm)
{
const struct gen_device_info *devinfo = disasm->devinfo;
const char *last_annotation_string = NULL;
@@ -49,47 +49,47 @@ dump_assembly(void *assembly, struct disasm_info *disasm)
int end_offset = next->offset;

if (group->block_start) {
- fprintf(stderr, " START B%d", group->block_start->num);
+ fprintf(fp, " START B%d", group->block_start->num);
foreach_list_typed(struct bblock_link, predecessor_link, link,
&group->block_start->parents) {
struct bblock_t *predecessor_block = predecessor_link->block;
- fprintf(stderr, " <-B%d", predecessor_block->num);
+ fprintf(fp, " <-B%d", predecessor_block->num);
}
- fprintf(stderr, " (%u cycles)\n", group->block_start->cycle_count);
+ fprintf(fp, " (%u cycles)\n", group->block_start->cycle_count);
}

if (last_annotation_ir != group->ir) {
last_annotation_ir = group->ir;
if (last_annotation_ir) {
- fprintf(stderr, " ");
- nir_print_instr(group->ir, stderr);
- fprintf(stderr, "\n");
+ fprintf(fp, " ");
+ nir_print_instr(group->ir, fp);
+ fprintf(fp, "\n");
}
}

if (last_annotation_string != group->annotation) {
last_annotation_string = group->annotation;
if (last_annotation_string)
- fprintf(stderr, " %s\n", last_annotation_string);
+ fprintf(fp, " %s\n", last_annotation_string);
}

- brw_disassemble(devinfo, assembly, start_offset, end_offset, stderr);
+ brw_disassemble(devinfo, assembly, start_offset, end_offset, fp);

if (group->error) {
- fputs(group->error, stderr);
+ fputs(group->error, fp);
}

if (group->block_end) {
- fprintf(stderr, " END B%d", group->block_end->num);
+ fprintf(fp, " END B%d", group->block_end->num);
foreach_list_typed(struct bblock_link, successor_link, link,
&group->block_end->children) {
struct bblock_t *successor_block = successor_link->block;
- fprintf(stderr, " ->B%d", successor_block->num);
+ fprintf(fp, " ->B%d", successor_block->num);
}
- fprintf(stderr, "\n");
+ fprintf(fp, "\n");
}
}
- fprintf(stderr, "\n");
+ fprintf(fp, "\n");
}

struct disasm_info *
diff --git a/src/intel/compiler/brw_disasm_info.h b/src/intel/compiler/brw_disasm_info.h
index b8826e68175..afdc6bc24f7 100644
--- a/src/intel/compiler/brw_disasm_info.h
+++ b/src/intel/compiler/brw_disasm_info.h
@@ -65,7 +65,7 @@ struct disasm_info {
};

void
-dump_assembly(void *assembly, struct disasm_info *disasm);
+dump_assembly(FILE *fp, void *assembly, struct disasm_info *disasm);

struct disasm_info *
disasm_initialize(const struct gen_device_info *devinfo,
diff --git a/src/intel/compiler/brw_fs_generator.cpp b/src/intel/compiler/brw_fs_generator.cpp
index 08dd83dded7..ebf63b07837 100644
--- a/src/intel/compiler/brw_fs_generator.cpp
+++ b/src/intel/compiler/brw_fs_generator.cpp
@@ -2471,7 +2471,7 @@ fs_generator::generate_code(const cfg_t *cfg, int dispatch_width)
spill_count, fill_count, promoted_constants, before_size, after_size,
100.0f * (before_size - after_size) / before_size);

- dump_assembly(p->store, disasm_info);
+ dump_assembly(stderr, p->store, disasm_info);
}
ralloc_free(disasm_info);
assert(validated);
diff --git a/src/intel/compiler/brw_vec4_generator.cpp b/src/intel/compiler/brw_vec4_generator.cpp
index 888cb358fd1..ee232cbf96e 100644
--- a/src/intel/compiler/brw_vec4_generator.cpp
+++ b/src/intel/compiler/brw_vec4_generator.cpp
@@ -2206,7 +2206,7 @@ generate_code(struct brw_codegen *p,
spill_count, fill_count, before_size, after_size,
100.0f * (before_size - after_size) / before_size);

- dump_assembly(p->store, disasm_info);
+ dump_assembly(stderr, p->store, disasm_info);
}
ralloc_free(disasm_info);
assert(validated);
diff --git a/src/intel/compiler/test_eu_validate.cpp b/src/intel/compiler/test_eu_validate.cpp
index 73300b23122..307245a00cc 100644
--- a/src/intel/compiler/test_eu_validate.cpp
+++ b/src/intel/compiler/test_eu_validate.cpp
@@ -107,7 +107,7 @@ validate(struct brw_codegen *p)
p->next_insn_offset, disasm);

if (print) {
- dump_assembly(p->store, disasm);
+ dump_assembly(stderr, p->store, disasm);
}
ralloc_free(disasm);
--
2.19.2
Mark Janes
2018-12-07 00:35:49 UTC
Permalink
---
src/intel/compiler/brw_fs.cpp | 11 ++++-----
src/intel/compiler/brw_nir.c | 26 +++++++++++++++++-----
src/intel/compiler/brw_nir.h | 1 +
src/intel/compiler/brw_shader.cpp | 2 +-
src/intel/compiler/brw_vec4.cpp | 2 +-
src/intel/compiler/brw_vec4_gs_visitor.cpp | 2 +-
src/intel/compiler/brw_vec4_tcs.cpp | 2 +-
7 files changed, 32 insertions(+), 14 deletions(-)

diff --git a/src/intel/compiler/brw_fs.cpp b/src/intel/compiler/brw_fs.cpp
index 43b920ae33d..f361d7aad9d 100644
--- a/src/intel/compiler/brw_fs.cpp
+++ b/src/intel/compiler/brw_fs.cpp
@@ -7139,7 +7139,7 @@ brw_compile_fs(const struct brw_compiler *compiler, void *log_data,
if (!key->multisample_fbo)
NIR_PASS_V(shader, demote_sample_qualifiers);
NIR_PASS_V(shader, move_interpolation_to_top);
- shader = brw_postprocess_nir(shader, compiler, true);
+ shader = brw_postprocess_nir(shader, compiler, log_data, true);

/* key->alpha_test_func means simulating alpha testing via discards,
* so the shader definitely kills pixels.
@@ -7384,6 +7384,7 @@ cs_set_simd_size(struct brw_cs_prog_data *cs_prog_data, unsigned size)

static nir_shader *
compile_cs_to_nir(const struct brw_compiler *compiler,
+ void *log_data,
void *mem_ctx,
const struct brw_cs_prog_key *key,
const nir_shader *src_shader,
@@ -7392,7 +7393,7 @@ compile_cs_to_nir(const struct brw_compiler *compiler,
nir_shader *shader = nir_shader_clone(mem_ctx, src_shader);
shader = brw_nir_apply_sampler_key(shader, compiler, &key->tex, true);
brw_nir_lower_cs_intrinsics(shader, dispatch_width);
- return brw_postprocess_nir(shader, compiler, true);
+ return brw_postprocess_nir(shader, compiler, log_data, true);
}

const unsigned *
@@ -7425,7 +7426,7 @@ brw_compile_cs(const struct brw_compiler *compiler, void *log_data,
/* Now the main event: Visit the shader IR and generate our CS IR for it.
*/
if (min_dispatch_width <= 8) {
- nir_shader *nir8 = compile_cs_to_nir(compiler, mem_ctx, key,
+ nir_shader *nir8 = compile_cs_to_nir(compiler, log_data, mem_ctx, key,
src_shader, 8);
v8 = new fs_visitor(compiler, log_data, mem_ctx, key, &prog_data->base,
NULL, /* Never used in core profile */
@@ -7446,7 +7447,7 @@ brw_compile_cs(const struct brw_compiler *compiler, void *log_data,
if (likely(!(INTEL_DEBUG & DEBUG_NO16)) &&
!fail_msg && min_dispatch_width <= 16) {
/* Try a SIMD16 compile */
- nir_shader *nir16 = compile_cs_to_nir(compiler, mem_ctx, key,
+ nir_shader *nir16 = compile_cs_to_nir(compiler, mem_ctx, log_data, key,
src_shader, 16);
v16 = new fs_visitor(compiler, log_data, mem_ctx, key, &prog_data->base,
NULL, /* Never used in core profile */
@@ -7479,7 +7480,7 @@ brw_compile_cs(const struct brw_compiler *compiler, void *log_data,

if (!fail_msg && (min_dispatch_width > 16 || (INTEL_DEBUG & DEBUG_DO32))) {
/* Try a SIMD32 compile */
- nir_shader *nir32 = compile_cs_to_nir(compiler, mem_ctx, key,
+ nir_shader *nir32 = compile_cs_to_nir(compiler, log_data, mem_ctx, key,
src_shader, 32);
v32 = new fs_visitor(compiler, log_data, mem_ctx, key, &prog_data->base,
NULL, /* Never used in core profile */
diff --git a/src/intel/compiler/brw_nir.c b/src/intel/compiler/brw_nir.c
index aa6788b9fe5..9c7877b920f 100644
--- a/src/intel/compiler/brw_nir.c
+++ b/src/intel/compiler/brw_nir.c
@@ -775,7 +775,7 @@ brw_nir_link_shaders(const struct brw_compiler *compiler,
*/
nir_shader *
brw_postprocess_nir(nir_shader *nir, const struct brw_compiler *compiler,
- bool is_scalar)
+ void *log_data, bool is_scalar)
{
const struct gen_device_info *devinfo = compiler->devinfo;
bool debug_enabled =
@@ -812,9 +812,17 @@ brw_postprocess_nir(nir_shader *nir, const struct brw_compiler *compiler,
nir_index_ssa_defs(function->impl);
}

- fprintf(stderr, "NIR (SSA form) for %s shader:\n",
+ char *buf;
+ size_t buf_size;
+ FILE * log_fp = open_memstream(&buf, &buf_size);
+ fprintf(log_fp, "NIR (SSA form) for %s shader:\n",
_mesa_shader_stage_to_string(nir->info.stage));
- nir_print_shader(nir, stderr);
+ nir_print_shader(nir, log_fp);
+ fclose(log_fp);
+ static GLuint msg_id = 0;
+ compiler->shader_debug_log(log_data, &msg_id, buf);
+ fputs(buf, stderr);
+ free(buf);
}

OPT(nir_convert_from_ssa, true);
@@ -837,9 +845,17 @@ brw_postprocess_nir(nir_shader *nir, const struct brw_compiler *compiler,
nir_sweep(nir);

if (unlikely(debug_enabled)) {
- fprintf(stderr, "NIR (final form) for %s shader:\n",
+ char *buf;
+ size_t buf_size;
+ FILE * log_fp = open_memstream(&buf, &buf_size);
+ fprintf(log_fp, "NIR (final form) for %s shader:\n",
_mesa_shader_stage_to_string(nir->info.stage));
- nir_print_shader(nir, stderr);
+ nir_print_shader(nir, log_fp);
+ fclose(log_fp);
+ static GLuint msg_id = 0;
+ compiler->shader_debug_log(log_data, &msg_id, buf);
+ fputs(buf, stderr);
+ free(buf);
}

return nir;
diff --git a/src/intel/compiler/brw_nir.h b/src/intel/compiler/brw_nir.h
index bc81950d47e..dda353a1665 100644
--- a/src/intel/compiler/brw_nir.h
+++ b/src/intel/compiler/brw_nir.h
@@ -123,6 +123,7 @@ bool brw_nir_lower_mem_access_bit_sizes(nir_shader *shader);

nir_shader *brw_postprocess_nir(nir_shader *nir,
const struct brw_compiler *compiler,
+ void *log_data,
bool is_scalar);

bool brw_nir_apply_attribute_workarounds(nir_shader *nir,
diff --git a/src/intel/compiler/brw_shader.cpp b/src/intel/compiler/brw_shader.cpp
index b77bd798d17..598a84f365a 100644
--- a/src/intel/compiler/brw_shader.cpp
+++ b/src/intel/compiler/brw_shader.cpp
@@ -1204,7 +1204,7 @@ brw_compile_tes(const struct brw_compiler *compiler,
nir = brw_nir_apply_sampler_key(nir, compiler, &key->tex, is_scalar);
brw_nir_lower_tes_inputs(nir, input_vue_map);
brw_nir_lower_vue_outputs(nir);
- nir = brw_postprocess_nir(nir, compiler, is_scalar);
+ nir = brw_postprocess_nir(nir, compiler, log_data, is_scalar);

brw_compute_vue_map(devinfo, &prog_data->base.vue_map,
nir->info.outputs_written,
diff --git a/src/intel/compiler/brw_vec4.cpp b/src/intel/compiler/brw_vec4.cpp
index 6ed07b72f31..13a64e308bf 100644
--- a/src/intel/compiler/brw_vec4.cpp
+++ b/src/intel/compiler/brw_vec4.cpp
@@ -2857,7 +2857,7 @@ brw_compile_vs(const struct brw_compiler *compiler, void *log_data,

brw_nir_lower_vs_inputs(shader, key->gl_attrib_wa_flags);
brw_nir_lower_vue_outputs(shader);
- shader = brw_postprocess_nir(shader, compiler, is_scalar);
+ shader = brw_postprocess_nir(shader, compiler, log_data, is_scalar);

prog_data->base.clip_distance_mask =
((1 << shader->info.clip_distance_array_size) - 1);
diff --git a/src/intel/compiler/brw_vec4_gs_visitor.cpp b/src/intel/compiler/brw_vec4_gs_visitor.cpp
index a6e38b0f379..db8bb298c81 100644
--- a/src/intel/compiler/brw_vec4_gs_visitor.cpp
+++ b/src/intel/compiler/brw_vec4_gs_visitor.cpp
@@ -642,7 +642,7 @@ brw_compile_gs(const struct brw_compiler *compiler, void *log_data,
shader = brw_nir_apply_sampler_key(shader, compiler, &key->tex, is_scalar);
brw_nir_lower_vue_inputs(shader, &c.input_vue_map);
brw_nir_lower_vue_outputs(shader);
- shader = brw_postprocess_nir(shader, compiler, is_scalar);
+ shader = brw_postprocess_nir(shader, compiler, log_data, is_scalar);

prog_data->base.clip_distance_mask =
((1 << shader->info.clip_distance_array_size) - 1);
diff --git a/src/intel/compiler/brw_vec4_tcs.cpp b/src/intel/compiler/brw_vec4_tcs.cpp
index be0969dda12..2528088ce8c 100644
--- a/src/intel/compiler/brw_vec4_tcs.cpp
+++ b/src/intel/compiler/brw_vec4_tcs.cpp
@@ -404,7 +404,7 @@ brw_compile_tcs(const struct brw_compiler *compiler,
if (key->quads_workaround)
brw_nir_apply_tcs_quads_workaround(nir);

- nir = brw_postprocess_nir(nir, compiler, is_scalar);
+ nir = brw_postprocess_nir(nir, compiler, log_data, is_scalar);

if (is_scalar)
prog_data->instances = DIV_ROUND_UP(nir->info.tess.tcs_vertices_out, 8);
--
2.19.2
Mark Janes
2018-12-07 00:35:51 UTC
Permalink
---
src/mesa/drivers/dri/i965/brw_link.cpp | 14 +++++++++++---
1 file changed, 11 insertions(+), 3 deletions(-)

diff --git a/src/mesa/drivers/dri/i965/brw_link.cpp b/src/mesa/drivers/dri/i965/brw_link.cpp
index 2cbb1e0b879..24e0079fc5d 100644
--- a/src/mesa/drivers/dri/i965/brw_link.cpp
+++ b/src/mesa/drivers/dri/i965/brw_link.cpp
@@ -251,10 +251,18 @@ brw_link_shader(struct gl_context *ctx, struct gl_shader_program *shProg)
(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",
+ char *buf;
+ size_t buf_size;
+ FILE * log_fp = open_memstream(&buf, &buf_size);
+ fprintf(log_fp, "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");
+ _mesa_print_ir(log_fp, shader->ir, NULL);
+ fprintf(log_fp, "\n\n");
+ fclose(log_fp);
+ static GLuint msg_id = 0;
+ shader_debug_log_mesa(brw, &msg_id, buf);
+ fputs(buf, stderr);
+ free(buf);
}

prog->nir = brw_create_nir(brw, shProg, prog, (gl_shader_stage) stage,
--
2.19.2
Mark Janes
2018-12-07 00:35:47 UTC
Permalink
Shader assemblies can be more easily incorporated into debug tools if
they are presented through KHR_debug.
---
src/intel/compiler/brw_fs_generator.cpp | 38 ++++++++++++-----------
src/intel/compiler/brw_vec4_generator.cpp | 22 +++++++------
2 files changed, 33 insertions(+), 27 deletions(-)

diff --git a/src/intel/compiler/brw_fs_generator.cpp b/src/intel/compiler/brw_fs_generator.cpp
index a8618912cf4..6b80f8433b2 100644
--- a/src/intel/compiler/brw_fs_generator.cpp
+++ b/src/intel/compiler/brw_fs_generator.cpp
@@ -2463,30 +2463,32 @@ fs_generator::generate_code(const cfg_t *cfg, int dispatch_width)
brw_compact_instructions(p, start_offset, disasm_info);
int after_size = p->next_insn_offset - start_offset;

+ char *buf;
+ size_t buf_size;
+ FILE * log_fp = open_memstream(&buf, &buf_size);
if (unlikely(debug_flag)) {
- fprintf(stderr, "Native code for %s\n"
- "SIMD%d shader: %d instructions. %d loops. %u cycles. %d:%d spills:fills. Promoted %u constants. Compacted %d to %d"
- " bytes (%.0f%%)\n",
- shader_name, dispatch_width, before_size / 16, loop_count, cfg->cycle_count,
- spill_count, fill_count, promoted_constants, before_size, after_size,
- 100.0f * (before_size - after_size) / before_size);
-
- dump_assembly(stderr, p->store, disasm_info);
+ fprintf(log_fp, "Native code for %s\n", shader_name);
}
+ fprintf(log_fp, "%s SIMD%d shader: %d inst, %d loops, %u cycles, "
+ "%d:%d spills:fills, Promoted %u constants, "
+ "compacted %d to %d bytes.",
+ _mesa_shader_stage_to_abbrev(stage),
+ dispatch_width, before_size / 16,
+ loop_count, cfg->cycle_count, spill_count,
+ fill_count, promoted_constants, before_size,
+ after_size);
+ if (unlikely(debug_flag)) {
+ dump_assembly(log_fp, p->store, disasm_info);
+ fflush(log_fp);
+ fputs(buf, stderr);
+ }
+ fclose(log_fp);
ralloc_free(disasm_info);
assert(validated);

static GLuint msg_id = 0;
- compiler->shader_debug_log(log_data, &msg_id,
- "%s SIMD%d shader: %d inst, %d loops, %u cycles, "
- "%d:%d spills:fills, Promoted %u constants, "
- "compacted %d to %d bytes.",
- _mesa_shader_stage_to_abbrev(stage),
- dispatch_width, before_size / 16,
- loop_count, cfg->cycle_count, spill_count,
- fill_count, promoted_constants, before_size,
- after_size);
-
+ compiler->shader_debug_log(log_data, &msg_id, "%s", buf);
+ free(buf);
return start_offset;
}

diff --git a/src/intel/compiler/brw_vec4_generator.cpp b/src/intel/compiler/brw_vec4_generator.cpp
index 00c89b21daf..f56ec6aa9a1 100644
--- a/src/intel/compiler/brw_vec4_generator.cpp
+++ b/src/intel/compiler/brw_vec4_generator.cpp
@@ -2195,30 +2195,34 @@ generate_code(struct brw_codegen *p,
brw_compact_instructions(p, 0, disasm_info);
int after_size = p->next_insn_offset;

+ char *buf;
+ size_t buf_size;
+ FILE * log_fp = open_memstream(&buf, &buf_size);
if (unlikely(debug_flag)) {
- fprintf(stderr, "Native code for %s %s shader %s:\n",
+ fprintf(log_fp, "Native code for %s %s shader %s:\n",
nir->info.label ? nir->info.label : "unnamed",
_mesa_shader_stage_to_string(nir->info.stage), nir->info.name);
+ }

- fprintf(stderr, "%s vec4 shader: %d instructions. %d loops. %u cycles. %d:%d "
+ fprintf(log_fp, "%s vec4 shader: %d instructions. %d loops. %u cycles. %d:%d "
"spills:fills. Compacted %d to %d bytes (%.0f%%)\n",
stage_abbrev, before_size / 16, loop_count, cfg->cycle_count,
spill_count, fill_count, before_size, after_size,
100.0f * (before_size - after_size) / before_size);

- dump_assembly(stderr, p->store, disasm_info);
+ if (unlikely(debug_flag)) {
+ dump_assembly(log_fp, p->store, disasm_info);
+ fflush(log_fp);
+ fputs(buf, stderr);
}
ralloc_free(disasm_info);
assert(validated);

+ fclose(log_fp);
static GLuint msg_id = 0;
compiler->shader_debug_log(log_data, &msg_id,
- "%s vec4 shader: %d inst, %d loops, %u cycles, "
- "%d:%d spills:fills, compacted %d to %d bytes.",
- stage_abbrev, before_size / 16,
- loop_count, cfg->cycle_count, spill_count,
- fill_count, before_size, after_size);
-
+ "%s", buf);
+ free(buf);
}

extern "C" const unsigned *
--
2.19.2
Mark Janes
2018-12-07 00:35:46 UTC
Permalink
KHR_debug indicates that message id's can be used to filter debug
logs. Mesa incorrectly generates incrementing message id's for each
message in shader_debug_log. The ids must be stable if they are to be
used for filtering.

_mesa_gl_vdebug expects the address of a static GLuint, which is
assigned at the first log event and remains stable for subsequent
events.
---
src/intel/compiler/brw_compiler.h | 2 +-
src/intel/compiler/brw_fs_generator.cpp | 3 ++-
src/intel/compiler/brw_vec4_generator.cpp | 3 ++-
src/intel/vulkan/anv_device.c | 2 +-
src/mesa/drivers/dri/i965/intel_screen.c | 5 ++---
5 files changed, 8 insertions(+), 7 deletions(-)

diff --git a/src/intel/compiler/brw_compiler.h b/src/intel/compiler/brw_compiler.h
index e4f4d83c8e8..ca9d41ca7a8 100644
--- a/src/intel/compiler/brw_compiler.h
+++ b/src/intel/compiler/brw_compiler.h
@@ -89,7 +89,7 @@ struct brw_compiler {
int aligned_pairs_class;
} fs_reg_sets[3];

- void (*shader_debug_log)(void *, const char *str, ...) PRINTFLIKE(2, 3);
+ void (*shader_debug_log)(void *, GLuint *msg_id, const char *str, ...) PRINTFLIKE(3, 4);
void (*shader_perf_log)(void *, const char *str, ...) PRINTFLIKE(2, 3);

bool scalar_stage[MESA_SHADER_STAGES];
diff --git a/src/intel/compiler/brw_fs_generator.cpp b/src/intel/compiler/brw_fs_generator.cpp
index ebf63b07837..a8618912cf4 100644
--- a/src/intel/compiler/brw_fs_generator.cpp
+++ b/src/intel/compiler/brw_fs_generator.cpp
@@ -2476,7 +2476,8 @@ fs_generator::generate_code(const cfg_t *cfg, int dispatch_width)
ralloc_free(disasm_info);
assert(validated);

- compiler->shader_debug_log(log_data,
+ static GLuint msg_id = 0;
+ compiler->shader_debug_log(log_data, &msg_id,
"%s SIMD%d shader: %d inst, %d loops, %u cycles, "
"%d:%d spills:fills, Promoted %u constants, "
"compacted %d to %d bytes.",
diff --git a/src/intel/compiler/brw_vec4_generator.cpp b/src/intel/compiler/brw_vec4_generator.cpp
index ee232cbf96e..00c89b21daf 100644
--- a/src/intel/compiler/brw_vec4_generator.cpp
+++ b/src/intel/compiler/brw_vec4_generator.cpp
@@ -2211,7 +2211,8 @@ generate_code(struct brw_codegen *p,
ralloc_free(disasm_info);
assert(validated);

- compiler->shader_debug_log(log_data,
+ static GLuint msg_id = 0;
+ compiler->shader_debug_log(log_data, &msg_id,
"%s vec4 shader: %d inst, %d loops, %u cycles, "
"%d:%d spills:fills, compacted %d to %d bytes.",
stage_abbrev, before_size / 16,
diff --git a/src/intel/vulkan/anv_device.c b/src/intel/vulkan/anv_device.c
index 6b5ba25c6bc..e60f7327ec1 100644
--- a/src/intel/vulkan/anv_device.c
+++ b/src/intel/vulkan/anv_device.c
@@ -45,7 +45,7 @@
#include "genxml/gen7_pack.h"

static void
-compiler_debug_log(void *data, const char *fmt, ...)
+compiler_debug_log(void *data, GLuint *id, const char *fmt, ...)
{ }

static void
diff --git a/src/mesa/drivers/dri/i965/intel_screen.c b/src/mesa/drivers/dri/i965/intel_screen.c
index c0201357869..6b54767f8dd 100644
--- a/src/mesa/drivers/dri/i965/intel_screen.c
+++ b/src/mesa/drivers/dri/i965/intel_screen.c
@@ -2389,14 +2389,13 @@ intel_device_get_revision(int fd)
}

static void
-shader_debug_log_mesa(void *data, const char *fmt, ...)
+shader_debug_log_mesa(void *data, GLuint *msg_id, const char *fmt, ...)
{
struct brw_context *brw = (struct brw_context *)data;
va_list args;

va_start(args, fmt);
- GLuint msg_id = 0;
- _mesa_gl_vdebugf(&brw->ctx, &msg_id,
+ _mesa_gl_vdebugf(&brw->ctx, msg_id,
MESA_DEBUG_SOURCE_SHADER_COMPILER,
MESA_DEBUG_TYPE_OTHER,
MESA_DEBUG_SEVERITY_NOTIFICATION, fmt, args);
--
2.19.2
Mark Janes
2018-12-07 00:35:50 UTC
Permalink
Debug output of IR/assemblies is likely to exceed the 4k limit in
KHR_debug.

shader_debug_log_mesa splits large messages, providing the full
content through KHR_debug. Expose the function, so it can be called
from the shader cache and the linker.
---
src/mesa/drivers/dri/i965/intel_screen.c | 2 +-
src/mesa/drivers/dri/i965/intel_screen.h | 3 +++
2 files changed, 4 insertions(+), 1 deletion(-)

diff --git a/src/mesa/drivers/dri/i965/intel_screen.c b/src/mesa/drivers/dri/i965/intel_screen.c
index ed0eaa783db..84f472814bc 100644
--- a/src/mesa/drivers/dri/i965/intel_screen.c
+++ b/src/mesa/drivers/dri/i965/intel_screen.c
@@ -2388,7 +2388,7 @@ intel_device_get_revision(int fd)
return revision;
}

-static void
+void
shader_debug_log_mesa(void *data, GLuint *msg_id, const char *buf)
{
struct brw_context *brw = (struct brw_context *)data;
diff --git a/src/mesa/drivers/dri/i965/intel_screen.h b/src/mesa/drivers/dri/i965/intel_screen.h
index 8d56fcd9e7a..8c1101d6b08 100644
--- a/src/mesa/drivers/dri/i965/intel_screen.h
+++ b/src/mesa/drivers/dri/i965/intel_screen.h
@@ -178,6 +178,9 @@ can_do_exec_capture(const struct intel_screen *screen)
return screen->kernel_features & KERNEL_ALLOWS_EXEC_CAPTURE;
}

+void
+shader_debug_log_mesa(void *data, GLuint *msg_id, const char *buf);
+
#ifdef __cplusplus
}
#endif
--
2.19.2
Mark Janes
2018-12-07 00:35:44 UTC
Permalink
---
src/mesa/main/errors.c | 27 +++++++++++++++++++++++++++
src/mesa/main/errors.h | 8 ++++++++
2 files changed, 35 insertions(+)

diff --git a/src/mesa/main/errors.c b/src/mesa/main/errors.c
index fad8cb59cae..995b0510575 100644
--- a/src/mesa/main/errors.c
+++ b/src/mesa/main/errors.c
@@ -253,6 +253,33 @@ _mesa_gl_debugf(struct gl_context *ctx,
va_end(args);
}

+size_t
+_mesa_gl_debug(struct gl_context *ctx,
+ GLuint *id,
+ enum mesa_debug_source source,
+ enum mesa_debug_type type,
+ enum mesa_debug_severity severity,
+ const char *msg)
+{
+ _mesa_debug_get_id(id);
+
+ size_t len = strnlen(msg, MAX_DEBUG_MESSAGE_LENGTH);
+ if (len < MAX_DEBUG_MESSAGE_LENGTH) {
+ _mesa_log_msg(ctx, source, type, *id, severity, len, msg);
+ return len;
+ }
+
+ /* limit the message to fit within KHR_debug buffers */
+ char s[MAX_DEBUG_MESSAGE_LENGTH];
+ strncpy(s, msg, MAX_DEBUG_MESSAGE_LENGTH);
+ s[MAX_DEBUG_MESSAGE_LENGTH - 1] = '\0';
+ len = MAX_DEBUG_MESSAGE_LENGTH - 1;
+ _mesa_log_msg(ctx, source, type, *id, severity, len, s);
+
+ /* report the number of characters that were logged */
+ return len;
+}
+

/**
* Record an OpenGL state error. These usually occur when the user
diff --git a/src/mesa/main/errors.h b/src/mesa/main/errors.h
index 3e2d56e7741..17fe380f26a 100644
--- a/src/mesa/main/errors.h
+++ b/src/mesa/main/errors.h
@@ -90,6 +90,14 @@ _mesa_gl_debugf(struct gl_context *ctx,
enum mesa_debug_severity severity,
const char *fmtString, ...) PRINTFLIKE(6, 7);

+extern size_t
+_mesa_gl_debug(struct gl_context *ctx,
+ GLuint *id,
+ enum mesa_debug_source source,
+ enum mesa_debug_type type,
+ enum mesa_debug_severity severity,
+ const char *msg);
+
#define _mesa_perf_debug(ctx, sev, ...) do { \
static GLuint msg_id = 0; \
if (unlikely(ctx->Const.ContextFlags & GL_CONTEXT_FLAG_DEBUG_BIT)) { \
--
2.19.2
Tapani Pälli
2018-12-10 10:30:48 UTC
Permalink
Post by Mark Janes
---
src/mesa/main/errors.c | 27 +++++++++++++++++++++++++++
src/mesa/main/errors.h | 8 ++++++++
2 files changed, 35 insertions(+)
diff --git a/src/mesa/main/errors.c b/src/mesa/main/errors.c
index fad8cb59cae..995b0510575 100644
--- a/src/mesa/main/errors.c
+++ b/src/mesa/main/errors.c
@@ -253,6 +253,33 @@ _mesa_gl_debugf(struct gl_context *ctx,
va_end(args);
}
+size_t
+_mesa_gl_debug(struct gl_context *ctx,
+ GLuint *id,
+ enum mesa_debug_source source,
+ enum mesa_debug_type type,
+ enum mesa_debug_severity severity,
+ const char *msg)
+{
+ _mesa_debug_get_id(id);
+
+ size_t len = strnlen(msg, MAX_DEBUG_MESSAGE_LENGTH);
+ if (len < MAX_DEBUG_MESSAGE_LENGTH) {
+ _mesa_log_msg(ctx, source, type, *id, severity, len, msg);
+ return len;
+ }
+
+ /* limit the message to fit within KHR_debug buffers */
+ char s[MAX_DEBUG_MESSAGE_LENGTH];
+ strncpy(s, msg, MAX_DEBUG_MESSAGE_LENGTH);
+ s[MAX_DEBUG_MESSAGE_LENGTH - 1] = '\0';
+ len = MAX_DEBUG_MESSAGE_LENGTH - 1;
+ _mesa_log_msg(ctx, source, type, *id, severity, len, s);
Maybe less code when using strndup here? Or maybe not, did not really
check but just throwing ideas :)
Post by Mark Janes
+
+ /* report the number of characters that were logged */
+ return len;
+}
+
/**
* Record an OpenGL state error. These usually occur when the user
diff --git a/src/mesa/main/errors.h b/src/mesa/main/errors.h
index 3e2d56e7741..17fe380f26a 100644
--- a/src/mesa/main/errors.h
+++ b/src/mesa/main/errors.h
@@ -90,6 +90,14 @@ _mesa_gl_debugf(struct gl_context *ctx,
enum mesa_debug_severity severity,
const char *fmtString, ...) PRINTFLIKE(6, 7);
+extern size_t
+_mesa_gl_debug(struct gl_context *ctx,
+ GLuint *id,
+ enum mesa_debug_source source,
+ enum mesa_debug_type type,
+ enum mesa_debug_severity severity,
+ const char *msg);
+
#define _mesa_perf_debug(ctx, sev, ...) do { \
static GLuint msg_id = 0; \
if (unlikely(ctx->Const.ContextFlags & GL_CONTEXT_FLAG_DEBUG_BIT)) { \
Ilia Mirkin
2018-12-07 00:53:37 UTC
Permalink
This series provides Intel shader compilation debug information via
KHR_debug. Previously, shader assembly and related compilation
artifacts were dumped to stderr. Tools associating compilation
artifacts with programs (e.g. FrameRetrace*) parsed stderr, which was
error prone. Changes to the shader debug formats and the addition of
shader cache assembly dumps further complicate the task of parsing
stderr.
KHR_debug provides synchronous callbacks with stable identifiers,
simplifying the task of matching debug artifacts with the originating
GLSL.
One observation is that while the identifiers may be stable within a
single execution, they will not be stable across different
applications / traces. id's are set on a first-come first-serve basis,
so depending on the exact order, the id's will end up different.

Is that OK for frameretrace? Another approach would be to create
globally-hardcoded id's for these, and start the auto-allocation in a
higher range.

-ilia
Mark Janes
2018-12-07 18:07:11 UTC
Permalink
Post by Ilia Mirkin
This series provides Intel shader compilation debug information via
KHR_debug. Previously, shader assembly and related compilation
artifacts were dumped to stderr. Tools associating compilation
artifacts with programs (e.g. FrameRetrace*) parsed stderr, which was
error prone. Changes to the shader debug formats and the addition of
shader cache assembly dumps further complicate the task of parsing
stderr.
KHR_debug provides synchronous callbacks with stable identifiers,
simplifying the task of matching debug artifacts with the originating
GLSL.
One observation is that while the identifiers may be stable within a
single execution, they will not be stable across different
applications / traces. id's are set on a first-come first-serve basis,
so depending on the exact order, the id's will end up different.
Is that OK for frameretrace? Another approach would be to create
globally-hardcoded id's for these, and start the auto-allocation in a
higher range.
I did take a few steps down the path of globally hard-coded id's. I
agree with Eric's assessment in debug_output.c that a giant enum list of
all debug message id's is unworkable.

We could divide id's into regions based on the high bits, and allocate
the lower bits to components to declare and manage. This would break up
the monolithic declaration of id's, but you still have the issue of id
stability as the driver changes over time. Refactoring functionality to
a location where it can be shared would involve changing the id's of
emitted debug messages. Also, any taxonomy used to split up the id's
will probably look shortsighted in 5 years time.

I personally haven't seen a great solution for sharing a 32 debug id
within a large project. The idea of exporting the id's to external
tools is even more problematic.

Rather than try to solve that problem, I decided to associate unknown
id's with the semantic meaning by parsing the first few words in
frameretrace. A stable id means subsequent messages can be handled
without parsing. Unknown messages are turned of via KHR_debug. This is
a much better parsing stderr, where it is difficult to determine message
boundaries.
Erik Faye-Lund
2018-12-07 09:33:36 UTC
Permalink
This series provides Intel shader compilation debug information via
KHR_debug. Previously, shader assembly and related compilation
artifacts were dumped to stderr. Tools associating compilation
artifacts with programs (e.g. FrameRetrace*) parsed stderr, which was
error prone. Changes to the shader debug formats and the addition of
shader cache assembly dumps further complicate the task of parsing
stderr.
KHR_debug provides synchronous callbacks with stable identifiers,
simplifying the task of matching debug artifacts with the originating
GLSL.
For backward compatibility, artifacts continue to be provided on
stderr. Pre-existing KHR_debug messages used by shader-db are
unchanged.
https://github.com/janesma/apitrace/tree/khr_debug
mesa: properly report the length of truncated log messages
mesa: rename logging functions to reflect that they format strings
mesa: add logging function for formatted string
i965/disasm: allow caller to specify output filehandle.
i965: provide stable message id's for shader_debug_log
i965/disasm: provide shader assembly to KHR_debug.
i965/compiler: provide formatted strings to the shader debug log
intel/nir: provide SSA form to KHR_debug
i965: make shader_debug_log_mesa callable
i965: provide GLSL IR to KHR_debug
i965: provide shader cache assemblies to KHR_debug
src/intel/compiler/brw_compiler.h | 2 +-
src/intel/compiler/brw_disasm_info.c | 28 +++++-----
src/intel/compiler/brw_disasm_info.h | 2 +-
src/intel/compiler/brw_fs.cpp | 11 ++--
src/intel/compiler/brw_fs_generator.cpp | 39 +++++++-------
src/intel/compiler/brw_nir.c | 26 +++++++--
src/intel/compiler/brw_nir.h | 1 +
src/intel/compiler/brw_shader.cpp | 2 +-
src/intel/compiler/brw_vec4.cpp | 2 +-
src/intel/compiler/brw_vec4_generator.cpp | 24 +++++----
src/intel/compiler/brw_vec4_gs_visitor.cpp | 2 +-
src/intel/compiler/brw_vec4_tcs.cpp | 2 +-
src/intel/compiler/test_eu_validate.cpp | 2 +-
src/intel/vulkan/anv_device.c | 2 +-
src/mesa/drivers/dri/i915/intel_context.h | 18 +++----
src/mesa/drivers/dri/i915/intel_fbo.c | 10 ++--
src/mesa/drivers/dri/i965/brw_context.h | 18 +++----
src/mesa/drivers/dri/i965/brw_disk_cache.c | 23 ++++++--
src/mesa/drivers/dri/i965/brw_link.cpp | 14 +++--
src/mesa/drivers/dri/i965/genX_state_upload.c | 14 ++---
src/mesa/drivers/dri/i965/intel_fbo.c | 10 ++--
src/mesa/drivers/dri/i965/intel_screen.c | 30 +++++------
src/mesa/drivers/dri/i965/intel_screen.h | 3 ++
src/mesa/main/bufferobj.c | 10 ++--
src/mesa/main/errors.c | 54 ++++++++++++++---
--
src/mesa/main/errors.h | 28 ++++++----
src/mesa/main/fbobject.c | 10 ++--
src/mesa/main/pipelineobj.c | 16 +++---
src/mesa/state_tracker/st_debug.c | 2 +-
29 files changed, 247 insertions(+), 158 deletions(-)
Loading...