From 5f93feed61a485a1f7cbd5c7f94bd94b93805e13 Mon Sep 17 00:00:00 2001 From: Alyssa Rosenzweig Date: Mon, 19 Sep 2022 11:06:12 -0400 Subject: [PATCH] panfrost: Don't merge workgroups with variable shared mem If nir->info.shared_size = 0 but grid->variable_shared_mem > 0, the shader uses shared memory but the compiler may not realize that. We need to disable workgroup merging even in this case. The alternate approach is to statically check for shared intrinsics in the compiler, but this is a bit easier all things considered. Signed-off-by: Alyssa Rosenzweig Part-of: --- src/gallium/drivers/panfrost/pan_cmdstream.c | 11 ++++++++++- src/panfrost/util/pan_ir.h | 5 ++++- 2 files changed, 14 insertions(+), 2 deletions(-) diff --git a/src/gallium/drivers/panfrost/pan_cmdstream.c b/src/gallium/drivers/panfrost/pan_cmdstream.c index 0d89e25245b..cdb793e0283 100644 --- a/src/gallium/drivers/panfrost/pan_cmdstream.c +++ b/src/gallium/drivers/panfrost/pan_cmdstream.c @@ -4158,7 +4158,16 @@ panfrost_launch_grid(struct pipe_context *pipe, batch->rsd[PIPE_SHADER_COMPUTE], panfrost_emit_shared_memory(batch, info)); - cfg.allow_merging_workgroups = cs->info.cs.allow_merging_workgroups; + /* Workgroups may be merged if the shader does not use barriers + * or shared memory. This condition is checked against the + * static shared_size at compile-time. We need to check the + * variable shared size at launch_grid time, because the + * compiler doesn't know about that. + */ + cfg.allow_merging_workgroups = + cs->info.cs.allow_merging_workgroups && + (info->variable_shared_mem == 0); + cfg.task_increment = 1; cfg.task_axis = MALI_TASK_AXIS_Z; } diff --git a/src/panfrost/util/pan_ir.h b/src/panfrost/util/pan_ir.h index 20915a1a6db..8b41668ea66 100644 --- a/src/panfrost/util/pan_ir.h +++ b/src/panfrost/util/pan_ir.h @@ -335,7 +335,10 @@ struct pan_shader_info { struct { /* Is it legal to merge workgroups? This is true if the - * shader uses neither barriers nor shared memory. + * shader uses neither barriers nor shared memory. This + * requires caution: if the API allows specifying shared + * memory at launch time (instead of compile time), that + * memory will not be accounted for by the compiler. * * Used by the Valhall hardware. */