Mesa (main): intel/compiler: Add id parameter to shader_debug_log callback
GitLab Mirror
gitlab-mirror at kemper.freedesktop.org
Mon Aug 2 00:18:31 UTC 2021
Module: Mesa
Branch: main
Commit: 043c5bf966a276c02c536846f44a1335e082789c
URL: http://cgit.freedesktop.org/mesa/mesa/commit/?id=043c5bf966a276c02c536846f44a1335e082789c
Author: Ian Romanick <ian.d.romanick at intel.com>
Date: Thu Jul 29 14:13:27 2021 -0700
intel/compiler: Add id parameter to shader_debug_log callback
There are two problems with the current architecture.
In OpenGL, the id is supposed to be a unique identifier for a particular
log source. This is done so that applications can (theoretically)
filter particular log messages. The debug callback infrastructure in
Mesa assigns a uniqe value when a value of 0 is passed in. This causes
the id to get set once to a unique value for each message.
By passing a stack variable that is initialized to 0 on every call,
every time the same message is logged, it will have a different id.
This isn't great, but it's also not catastrophic.
When threaded shader compiles are used, the id *pointer* is saved and
dereferenced at a possibly much later time on a possibly different
thread. This causes one thread to access the stack from a different
thread... and that stack frame might not be valid any more. :(
This fixes shader-db crashes of various kinds on Iris with threaded
shader compiles enabled.
Fixes: 42c34e1ac8d ("iris: Enable threaded shader compilation")
Reviewed-by: Matt Turner <mattst88 at gmail.com>
Reviewed-by: Anuj Phogat <anuj.phogat at gmail.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/12136>
---
src/gallium/drivers/crocus/crocus_screen.c | 5 ++---
src/gallium/drivers/iris/iris_screen.c | 5 ++---
src/intel/compiler/brw_compiler.h | 7 ++++++-
src/intel/compiler/brw_fs_generator.cpp | 26 +++++++++++++-------------
src/intel/compiler/brw_vec4_generator.cpp | 14 +++++++-------
src/intel/vulkan/anv_device.c | 2 +-
src/mesa/drivers/dri/i965/brw_screen.c | 5 ++---
7 files changed, 33 insertions(+), 31 deletions(-)
diff --git a/src/gallium/drivers/crocus/crocus_screen.c b/src/gallium/drivers/crocus/crocus_screen.c
index 2647fb923ee..9a29ada93bd 100644
--- a/src/gallium/drivers/crocus/crocus_screen.c
+++ b/src/gallium/drivers/crocus/crocus_screen.c
@@ -674,17 +674,16 @@ crocus_get_default_l3_config(const struct intel_device_info *devinfo,
}
static void
-crocus_shader_debug_log(void *data, const char *fmt, ...)
+crocus_shader_debug_log(void *data, unsigned *id, const char *fmt, ...)
{
struct pipe_debug_callback *dbg = data;
- unsigned id = 0;
va_list args;
if (!dbg->debug_message)
return;
va_start(args, fmt);
- dbg->debug_message(dbg->data, &id, PIPE_DEBUG_TYPE_SHADER_INFO, fmt, args);
+ dbg->debug_message(dbg->data, id, PIPE_DEBUG_TYPE_SHADER_INFO, fmt, args);
va_end(args);
}
diff --git a/src/gallium/drivers/iris/iris_screen.c b/src/gallium/drivers/iris/iris_screen.c
index ab868495ad6..612c0c6a5bd 100644
--- a/src/gallium/drivers/iris/iris_screen.c
+++ b/src/gallium/drivers/iris/iris_screen.c
@@ -717,17 +717,16 @@ iris_get_default_l3_config(const struct intel_device_info *devinfo,
}
static void
-iris_shader_debug_log(void *data, const char *fmt, ...)
+iris_shader_debug_log(void *data, unsigned *id, const char *fmt, ...)
{
struct pipe_debug_callback *dbg = data;
- unsigned id = 0;
va_list args;
if (!dbg->debug_message)
return;
va_start(args, fmt);
- dbg->debug_message(dbg->data, &id, PIPE_DEBUG_TYPE_SHADER_INFO, fmt, args);
+ dbg->debug_message(dbg->data, id, PIPE_DEBUG_TYPE_SHADER_INFO, fmt, args);
va_end(args);
}
diff --git a/src/intel/compiler/brw_compiler.h b/src/intel/compiler/brw_compiler.h
index e7c7278306f..5f07e9cc435 100644
--- a/src/intel/compiler/brw_compiler.h
+++ b/src/intel/compiler/brw_compiler.h
@@ -69,7 +69,7 @@ struct brw_compiler {
struct ra_class *aligned_bary_class;
} fs_reg_sets[3];
- void (*shader_debug_log)(void *, const char *str, ...) PRINTFLIKE(2, 3);
+ void (*shader_debug_log)(void *, unsigned *id, const char *str, ...) PRINTFLIKE(3, 4);
void (*shader_perf_log)(void *, const char *str, ...) PRINTFLIKE(2, 3);
bool scalar_stage[MESA_ALL_SHADER_STAGES];
@@ -121,6 +121,11 @@ struct brw_compiler {
bool indirect_ubos_use_sampler;
};
+#define brw_shader_debug_log(compiler, data, fmt, ... ) do { \
+ static unsigned id = 0; \
+ compiler->shader_debug_log(data, &id, fmt, ##__VA_ARGS__); \
+} while (0)
+
/**
* We use a constant subgroup size of 32. It really only needs to be a
* maximum and, since we do SIMD32 for compute shaders in some cases, it
diff --git a/src/intel/compiler/brw_fs_generator.cpp b/src/intel/compiler/brw_fs_generator.cpp
index 068e4191534..8740c7a65f3 100644
--- a/src/intel/compiler/brw_fs_generator.cpp
+++ b/src/intel/compiler/brw_fs_generator.cpp
@@ -2794,19 +2794,19 @@ fs_generator::generate_code(const cfg_t *cfg, int dispatch_width,
#endif
assert(validated);
- compiler->shader_debug_log(log_data,
- "%s SIMD%d shader: %d inst, %d loops, %u cycles, "
- "%d:%d spills:fills, %u sends, "
- "scheduled with mode %s, "
- "Promoted %u constants, "
- "compacted %d to %d bytes.",
- _mesa_shader_stage_to_abbrev(stage),
- dispatch_width, before_size / 16 - nop_count,
- loop_count, perf.latency,
- spill_count, fill_count, send_count,
- shader_stats.scheduler_mode,
- shader_stats.promoted_constants,
- before_size, after_size);
+ brw_shader_debug_log(compiler, log_data,
+ "%s SIMD%d shader: %d inst, %d loops, %u cycles, "
+ "%d:%d spills:fills, %u sends, "
+ "scheduled with mode %s, "
+ "Promoted %u constants, "
+ "compacted %d to %d bytes.",
+ _mesa_shader_stage_to_abbrev(stage),
+ dispatch_width, before_size / 16 - nop_count,
+ loop_count, perf.latency,
+ spill_count, fill_count, send_count,
+ shader_stats.scheduler_mode,
+ shader_stats.promoted_constants,
+ before_size, after_size);
if (stats) {
stats->dispatch_width = dispatch_width;
stats->instructions = before_size / 16 - nop_count;
diff --git a/src/intel/compiler/brw_vec4_generator.cpp b/src/intel/compiler/brw_vec4_generator.cpp
index 1d20d87b2a0..e1fb00fcfaa 100644
--- a/src/intel/compiler/brw_vec4_generator.cpp
+++ b/src/intel/compiler/brw_vec4_generator.cpp
@@ -2263,13 +2263,13 @@ generate_code(struct brw_codegen *p,
ralloc_free(disasm_info);
assert(validated);
- compiler->shader_debug_log(log_data,
- "%s vec4 shader: %d inst, %d loops, %u cycles, "
- "%d:%d spills:fills, %u sends, "
- "compacted %d to %d bytes.",
- stage_abbrev, before_size / 16,
- loop_count, perf.latency, spill_count,
- fill_count, send_count, before_size, after_size);
+ brw_shader_debug_log(compiler, log_data,
+ "%s vec4 shader: %d inst, %d loops, %u cycles, "
+ "%d:%d spills:fills, %u sends, "
+ "compacted %d to %d bytes.",
+ stage_abbrev, before_size / 16,
+ loop_count, perf.latency, spill_count,
+ fill_count, send_count, before_size, after_size);
if (stats) {
stats->dispatch_width = 0;
stats->instructions = before_size / 16;
diff --git a/src/intel/vulkan/anv_device.c b/src/intel/vulkan/anv_device.c
index 52d819aad6f..84c34e0e31f 100644
--- a/src/intel/vulkan/anv_device.c
+++ b/src/intel/vulkan/anv_device.c
@@ -85,7 +85,7 @@ static const driOptionDescription anv_dri_options[] = {
#endif
static void
-compiler_debug_log(void *data, const char *fmt, ...)
+compiler_debug_log(void *data, UNUSED unsigned *id, const char *fmt, ...)
{
char str[MAX_DEBUG_MESSAGE_LENGTH];
struct anv_device *device = (struct anv_device *)data;
diff --git a/src/mesa/drivers/dri/i965/brw_screen.c b/src/mesa/drivers/dri/i965/brw_screen.c
index dad4bc29463..20ca5c21451 100644
--- a/src/mesa/drivers/dri/i965/brw_screen.c
+++ b/src/mesa/drivers/dri/i965/brw_screen.c
@@ -2472,14 +2472,13 @@ set_max_gl_versions(struct brw_screen *screen)
}
static void
-shader_debug_log_mesa(void *data, const char *fmt, ...)
+shader_debug_log_mesa(void *data, unsigned *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);
More information about the mesa-commit
mailing list