From fe6a84729d485072898b112899e5f9fa57766853 Mon Sep 17 00:00:00 2001 From: Erik Faye-Lund Date: Wed, 2 Nov 2022 10:29:10 +0100 Subject: [PATCH] zink: put union fields into structs named by the shader-stages This makes it easier to see that a field is only valid in a given stage, to avoid undefined behavior. Part-of: --- src/gallium/drivers/zink/zink_compiler.c | 20 +++++++++------- src/gallium/drivers/zink/zink_draw.cpp | 3 ++- src/gallium/drivers/zink/zink_program.c | 24 +++++++++---------- .../drivers/zink/zink_program_state.hpp | 2 +- src/gallium/drivers/zink/zink_types.h | 14 ++++++++--- 5 files changed, 37 insertions(+), 26 deletions(-) diff --git a/src/gallium/drivers/zink/zink_compiler.c b/src/gallium/drivers/zink/zink_compiler.c index 9d1abd3be18..c59c4d5ae63 100644 --- a/src/gallium/drivers/zink/zink_compiler.c +++ b/src/gallium/drivers/zink/zink_compiler.c @@ -2348,7 +2348,7 @@ zink_shader_compile(struct zink_screen *screen, struct zink_shader *zs, nir_shad ralloc_free(nir); /* TODO: determine if there's any reason to cache spirv output? */ - if (zs->nir->info.stage == MESA_SHADER_TESS_CTRL && zs->is_generated) + if (zs->nir->info.stage == MESA_SHADER_TESS_CTRL && zs->tcs.is_generated) zs->spirv = spirv; else ralloc_free(spirv); @@ -3630,9 +3630,10 @@ zink_shader_free(struct zink_screen *screen, struct zink_shader *shader) gl_shader_stage stage = shader->nir->info.stage; assert(stage < ZINK_GFX_SHADER_COUNT); if (!prog->base.removed && prog->stages_present == prog->stages_remaining && - (stage != MESA_SHADER_TESS_CTRL || !shader->is_generated)) { + (stage != MESA_SHADER_TESS_CTRL || !shader->tcs.is_generated)) { unsigned stages_present = prog->stages_present; - if (prog->shaders[MESA_SHADER_TESS_CTRL] && prog->shaders[MESA_SHADER_TESS_CTRL]->is_generated) + if (prog->shaders[MESA_SHADER_TESS_CTRL] && + prog->shaders[MESA_SHADER_TESS_CTRL]->tcs.is_generated) stages_present &= ~BITFIELD_BIT(MESA_SHADER_TESS_CTRL); unsigned idx = zink_program_cache_stages(stages_present); struct hash_table *ht = &prog->ctx->program_cache[idx]; @@ -3644,19 +3645,20 @@ zink_shader_free(struct zink_screen *screen, struct zink_shader *shader) simple_mtx_unlock(&prog->ctx->program_lock[idx]); util_queue_fence_wait(&prog->base.cache_fence); } - if (stage != MESA_SHADER_TESS_CTRL || !shader->is_generated) { + if (stage != MESA_SHADER_TESS_CTRL || !shader->tcs.is_generated) { prog->shaders[stage] = NULL; prog->stages_remaining &= ~BITFIELD_BIT(stage); } /* only remove generated tcs during parent tes destruction */ - if (stage == MESA_SHADER_TESS_EVAL && shader->generated) + if (stage == MESA_SHADER_TESS_EVAL && shader->tes.generated) prog->shaders[MESA_SHADER_TESS_CTRL] = NULL; zink_gfx_program_reference(screen, &prog, NULL); } - if (shader->nir->info.stage == MESA_SHADER_TESS_EVAL && shader->generated) { + if (shader->nir->info.stage == MESA_SHADER_TESS_EVAL && + shader->tes.generated) { /* automatically destroy generated tcs shaders when tes is destroyed */ - zink_shader_free(screen, shader->generated); - shader->generated = NULL; + zink_shader_free(screen, shader->tes.generated); + shader->tes.generated = NULL; } _mesa_set_destroy(shader->programs, NULL); ralloc_free(shader->nir); @@ -3775,7 +3777,7 @@ zink_shader_tcs_create(struct zink_screen *screen, struct zink_shader *vs, unsig NIR_PASS_V(nir, nir_convert_from_ssa, true); ret->nir = nir; - ret->is_generated = true; + ret->tcs.is_generated = true; return ret; } diff --git a/src/gallium/drivers/zink/zink_draw.cpp b/src/gallium/drivers/zink/zink_draw.cpp index 40844d27995..1f1936e48a4 100644 --- a/src/gallium/drivers/zink/zink_draw.cpp +++ b/src/gallium/drivers/zink/zink_draw.cpp @@ -785,7 +785,8 @@ zink_draw(struct pipe_context *pctx, offsetof(struct zink_gfx_push_constant, draw_mode_is_indexed), sizeof(unsigned), &draw_mode_is_indexed); } - if (ctx->curr_program->shaders[MESA_SHADER_TESS_CTRL] && ctx->curr_program->shaders[MESA_SHADER_TESS_CTRL]->is_generated) { + if (ctx->curr_program->shaders[MESA_SHADER_TESS_CTRL] && + ctx->curr_program->shaders[MESA_SHADER_TESS_CTRL]->tcs.is_generated) { VKCTX(CmdPushConstants)(batch->state->cmdbuf, ctx->curr_program->base.layout, VK_SHADER_STAGE_ALL_GRAPHICS, offsetof(struct zink_gfx_push_constant, default_inner_level), sizeof(float) * 6, &ctx->tess_levels[0]); diff --git a/src/gallium/drivers/zink/zink_program.c b/src/gallium/drivers/zink/zink_program.c index 9a3bb8b85f5..66e2161b076 100644 --- a/src/gallium/drivers/zink/zink_program.c +++ b/src/gallium/drivers/zink/zink_program.c @@ -129,13 +129,13 @@ create_shader_module_for_stage(struct zink_context *ctx, struct zink_screen *scr struct zink_shader_module *zm; const struct zink_shader_key *key = &state->shader_keys.key[stage]; /* non-generated tcs won't use the shader key */ - const bool is_nongenerated_tcs = stage == MESA_SHADER_TESS_CTRL && !zs->is_generated; + const bool is_nongenerated_tcs = stage == MESA_SHADER_TESS_CTRL && !zs->tcs.is_generated; zm = malloc(sizeof(struct zink_shader_module) + key->size + (!has_nonseamless ? nonseamless_size : 0) + inline_size * sizeof(uint32_t)); if (!zm) { return NULL; } unsigned patch_vertices = state->shader_keys.key[MESA_SHADER_TESS_CTRL ].key.tcs.patch_vertices; - if (stage == MESA_SHADER_TESS_CTRL && zs->is_generated && zs->spirv) { + if (stage == MESA_SHADER_TESS_CTRL && zs->tcs.is_generated && zs->spirv) { assert(ctx); //TODO async mod = zink_shader_tcs_compile(screen, zs, patch_vertices); } else { @@ -161,7 +161,7 @@ create_shader_module_for_stage(struct zink_context *ctx, struct zink_screen *scr zm->has_nonseamless = has_nonseamless ? 0 : !!nonseamless_size; if (inline_size) memcpy(zm->key + key->size + nonseamless_size, key->base.inlined_uniform_values, inline_size * sizeof(uint32_t)); - if (stage == MESA_SHADER_TESS_CTRL && zs->is_generated) + if (stage == MESA_SHADER_TESS_CTRL && zs->tcs.is_generated) zm->hash = patch_vertices; else zm->hash = shader_module_hash(zm); @@ -183,7 +183,7 @@ get_shader_module_for_stage(struct zink_context *ctx, struct zink_screen *screen { const struct zink_shader_key *key = &state->shader_keys.key[stage]; /* non-generated tcs won't use the shader key */ - const bool is_nongenerated_tcs = stage == MESA_SHADER_TESS_CTRL && !zs->is_generated; + const bool is_nongenerated_tcs = stage == MESA_SHADER_TESS_CTRL && !zs->tcs.is_generated; struct util_dynarray *shader_cache = &prog->shader_cache[stage][!has_nonseamless ? !!nonseamless_size : 0][has_inline ? !!inline_size : 0]; unsigned count = util_dynarray_num_elements(shader_cache, struct zink_shader_module *); @@ -224,7 +224,7 @@ create_shader_module_for_stage_optimal(struct zink_context *ctx, struct zink_scr key = (uint16_t*)&state->shader_keys_optimal.key.vs_base; } else if (stage == MESA_SHADER_FRAGMENT) { key = (uint16_t*)&state->shader_keys_optimal.key.fs; - } else if (stage == MESA_SHADER_TESS_CTRL && zs->is_generated) { + } else if (stage == MESA_SHADER_TESS_CTRL && zs->tcs.is_generated) { key = (uint16_t*)&state->shader_keys_optimal.key.tcs; } else { key = NULL; @@ -234,7 +234,7 @@ create_shader_module_for_stage_optimal(struct zink_context *ctx, struct zink_scr if (!zm) { return NULL; } - if (stage == MESA_SHADER_TESS_CTRL && zs->is_generated && zs->spirv) { + if (stage == MESA_SHADER_TESS_CTRL && zs->tcs.is_generated && zs->spirv) { assert(ctx); //TODO async struct zink_tcs_key *tcs = (struct zink_tcs_key*)key; mod = zink_shader_tcs_compile(screen, zs, tcs->patch_vertices); @@ -247,7 +247,7 @@ create_shader_module_for_stage_optimal(struct zink_context *ctx, struct zink_scr } zm->shader = mod; /* non-generated tcs won't use the shader key */ - const bool is_nongenerated_tcs = stage == MESA_SHADER_TESS_CTRL && !zs->is_generated; + const bool is_nongenerated_tcs = stage == MESA_SHADER_TESS_CTRL && !zs->tcs.is_generated; if (key && !is_nongenerated_tcs) { zm->key_size = key_size; uint16_t *data = (uint16_t*)zm->key; @@ -266,14 +266,14 @@ get_shader_module_for_stage_optimal(struct zink_context *ctx, struct zink_screen struct zink_gfx_pipeline_state *state) { /* non-generated tcs won't use the shader key */ - const bool is_nongenerated_tcs = stage == MESA_SHADER_TESS_CTRL && !zs->is_generated; + const bool is_nongenerated_tcs = stage == MESA_SHADER_TESS_CTRL && !zs->tcs.is_generated; uint16_t *key; unsigned mask = stage == MESA_SHADER_FRAGMENT ? BITFIELD_MASK(16) : BITFIELD_MASK(8); if (zs == prog->last_vertex_stage) { key = (uint16_t*)&ctx->gfx_pipeline_state.shader_keys_optimal.key.vs_base; } else if (stage == MESA_SHADER_FRAGMENT) { key = (uint16_t*)&ctx->gfx_pipeline_state.shader_keys_optimal.key.fs; - } else if (stage == MESA_SHADER_TESS_CTRL && zs->is_generated) { + } else if (stage == MESA_SHADER_TESS_CTRL && zs->tcs.is_generated) { key = (uint16_t*)&ctx->gfx_pipeline_state.shader_keys_optimal.key.tcs; } else { key = NULL; @@ -627,7 +627,7 @@ update_gfx_program_optimal(struct zink_context *ctx, struct zink_gfx_program *pr update_gfx_shader_module_optimal(ctx, prog, MESA_SHADER_FRAGMENT); ctx->gfx_pipeline_state.modules_changed = true; } - if (prog->shaders[MESA_SHADER_TESS_CTRL] && prog->shaders[MESA_SHADER_TESS_CTRL]->is_generated && + if (prog->shaders[MESA_SHADER_TESS_CTRL] && prog->shaders[MESA_SHADER_TESS_CTRL]->tcs.is_generated && ctx->gfx_pipeline_state.shader_keys_optimal.key.tcs_bits != optimal_key->tcs_bits) { update_gfx_shader_module_optimal(ctx, prog, MESA_SHADER_TESS_CTRL); ctx->gfx_pipeline_state.modules_changed = true; @@ -876,7 +876,7 @@ zink_create_gfx_program(struct zink_context *ctx, } } if (stages[MESA_SHADER_TESS_EVAL] && !stages[MESA_SHADER_TESS_CTRL]) { - prog->shaders[MESA_SHADER_TESS_EVAL]->generated = + prog->shaders[MESA_SHADER_TESS_EVAL]->tes.generated = prog->shaders[MESA_SHADER_TESS_CTRL] = zink_shader_tcs_create(screen, stages[MESA_SHADER_VERTEX], vertices_per_patch); prog->stages_present |= BITFIELD_BIT(MESA_SHADER_TESS_CTRL); @@ -1496,7 +1496,7 @@ zink_bind_tes_state(struct pipe_context *pctx, if (!!ctx->gfx_stages[MESA_SHADER_TESS_EVAL] != !!cso) { if (!cso) { /* if unsetting a TESS that uses a generated TCS, ensure the TCS is unset */ - if (ctx->gfx_stages[MESA_SHADER_TESS_EVAL]->generated) + if (ctx->gfx_stages[MESA_SHADER_TESS_EVAL]->tes.generated) ctx->gfx_stages[MESA_SHADER_TESS_CTRL] = NULL; } } diff --git a/src/gallium/drivers/zink/zink_program_state.hpp b/src/gallium/drivers/zink/zink_program_state.hpp index 687287fe33c..d1832f4a413 100644 --- a/src/gallium/drivers/zink/zink_program_state.hpp +++ b/src/gallium/drivers/zink/zink_program_state.hpp @@ -395,7 +395,7 @@ get_gfx_pipeline_stage_eq_func(struct zink_gfx_program *prog, bool optimal_keys) { unsigned vertex_stages = prog->stages_present & BITFIELD_MASK(MESA_SHADER_FRAGMENT); if (vertex_stages & BITFIELD_BIT(MESA_SHADER_TESS_CTRL)) { - if (prog->shaders[MESA_SHADER_TESS_CTRL]->is_generated) + if (prog->shaders[MESA_SHADER_TESS_CTRL]->tcs.is_generated) vertex_stages &= ~BITFIELD_BIT(MESA_SHADER_TESS_CTRL); } if (vertex_stages & BITFIELD_BIT(MESA_SHADER_TESS_CTRL)) { diff --git a/src/gallium/drivers/zink/zink_types.h b/src/gallium/drivers/zink/zink_types.h index aa258bf467f..e1b96ffe3ef 100644 --- a/src/gallium/drivers/zink/zink_types.h +++ b/src/gallium/drivers/zink/zink_types.h @@ -687,9 +687,17 @@ struct zink_shader { struct set *programs; union { - struct zink_shader *generated; // a generated shader that this shader "owns" - bool is_generated; // if this is a driver-created shader (e.g., tcs) - nir_variable *fbfetch; //for fs output + struct { + struct zink_shader *generated; // a generated shader that this shader "owns" + } tes; + + struct { + bool is_generated; // if this is a driver-created shader (e.g., tcs) + } tcs; + + struct { + nir_variable *fbfetch; //for fs output + } fs; }; };