Mesa (master): iris: Refcount shader variants
GitLab Mirror
gitlab-mirror at kemper.freedesktop.org
Fri Jan 29 06:33:34 UTC 2021
Module: Mesa
Branch: master
Commit: 578cd00d93624eee4654d16656a44fd5ffd0527f
URL: http://cgit.freedesktop.org/mesa/mesa/commit/?id=578cd00d93624eee4654d16656a44fd5ffd0527f
Author: Kenneth Graunke <kenneth at whitecape.org>
Date: Wed Jan 27 13:46:26 2021 -0800
iris: Refcount shader variants
There is a small gap of time where the currently bound uncompiled
shaders, and compiled shader variant, are out of sync. Specifically,
between pipe->bind_*_state() and the next draw.
Currently, shaders variants live entirely within a single context,
and when deleting an iris_uncompiled_shader, we check if any of its
variants are currently bound, and defer deleting those until the next
iris_update_compiled_shaders() hook runs and binds new shaders to
replace them. (This is due to the time gap between binding new
uncompiled shaders, and updating variants at draw time when we have
the required NOS in place.)
This works pretty well in a single context world. But as we move to
share compiled shader variants across multiple contexts, it breaks down.
When deleting a shader, we can't look at all contexts to see if its
variants are bound anywhere. We can't even quantify whether those
contexts will run a future draw any time soon, to update and unbind.
One fairly crazy solution would be to delete the variants anyway, and
leave the stale pointers to dead variants in place. This requires
removing any code that compares old and new variants. Today, we do
that sometimes for seeing if the old/new shaders toggled some feature.
Worse than that, though, we don't just have to avoid dereferences, we'd
have to avoid pointer comparisons. If we free a variant, and quickly
allocate a new variant, malloc may return the same pointer. If it's
for the same shader stage, we may get a new different program that has
the same pointer as a previously bound stale one, causing us to think
nothing had changed when we really needed to do updates. Again, this
is doable, but leaves the code fragile - we'd have to guard against
future patches adding such checks back in.
So, don't do that. Instead, do basic reference counting. When a
variant is bound in a context, up the reference. When it's unbound,
decrement it. When it hits zero, we know it's not bound anywhere and
is safe to delete, with no stale references. This ends up being
reasonably cheap anyway, since the atomic is usually uncontested.
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/7668>
---
src/gallium/drivers/iris/iris_context.h | 21 ++++++---
src/gallium/drivers/iris/iris_program.c | 24 ++++++----
src/gallium/drivers/iris/iris_program_cache.c | 68 +++------------------------
3 files changed, 36 insertions(+), 77 deletions(-)
diff --git a/src/gallium/drivers/iris/iris_context.h b/src/gallium/drivers/iris/iris_context.h
index ffdd96ec922..5fd4cdb5e11 100644
--- a/src/gallium/drivers/iris/iris_context.h
+++ b/src/gallium/drivers/iris/iris_context.h
@@ -429,7 +429,7 @@ struct iris_binding_table {
* (iris_uncompiled_shader), due to state-based recompiles (brw_*_prog_key).
*/
struct iris_compiled_shader {
- struct list_head link;
+ struct pipe_reference ref;
/** Reference to the uploaded assembly. */
struct iris_state_ref assembly;
@@ -615,9 +615,6 @@ struct iris_context {
bool constrained;
} urb;
- /** List of shader variants whose deletion has been deferred for now */
- struct list_head deleted_variants[MESA_SHADER_STAGES];
-
struct u_upload_mgr *uploader;
struct hash_table *cache;
@@ -911,8 +908,20 @@ struct iris_compiled_shader *iris_upload_shader(struct iris_context *ice,
const void *iris_find_previous_compile(const struct iris_context *ice,
enum iris_program_cache_id cache_id,
unsigned program_string_id);
-void iris_delete_shader_variants(struct iris_context *ice,
- struct iris_uncompiled_shader *ish);
+void iris_delete_shader_variant(struct iris_compiled_shader *shader);
+
+static inline void
+iris_shader_variant_reference(struct iris_compiled_shader **dst,
+ struct iris_compiled_shader *src)
+{
+ struct iris_compiled_shader *old_dst = *dst;
+
+ if (pipe_reference(old_dst ? &old_dst->ref: NULL, src ? &src->ref : NULL))
+ iris_delete_shader_variant(old_dst);
+
+ *dst = src;
+}
+
bool iris_blorp_lookup_shader(struct blorp_batch *blorp_batch,
const void *key,
uint32_t key_size,
diff --git a/src/gallium/drivers/iris/iris_program.c b/src/gallium/drivers/iris/iris_program.c
index acf4a8e3874..c53b808b600 100644
--- a/src/gallium/drivers/iris/iris_program.c
+++ b/src/gallium/drivers/iris/iris_program.c
@@ -1213,7 +1213,8 @@ iris_update_compiled_vs(struct iris_context *ice)
shader = iris_compile_vs(ice, ish, &key);
if (old != shader) {
- ice->shaders.prog[IRIS_CACHE_VS] = shader;
+ iris_shader_variant_reference(&ice->shaders.prog[MESA_SHADER_VERTEX],
+ shader);
ice->state.dirty |= IRIS_DIRTY_VF_SGVS;
ice->state.stage_dirty |= IRIS_STAGE_DIRTY_VS |
IRIS_STAGE_DIRTY_BINDINGS_VS |
@@ -1411,7 +1412,8 @@ iris_update_compiled_tcs(struct iris_context *ice)
shader = iris_compile_tcs(ice, tcs, &key);
if (old != shader) {
- ice->shaders.prog[IRIS_CACHE_TCS] = shader;
+ iris_shader_variant_reference(&ice->shaders.prog[MESA_SHADER_TESS_CTRL],
+ shader);
ice->state.stage_dirty |= IRIS_STAGE_DIRTY_TCS |
IRIS_STAGE_DIRTY_BINDINGS_TCS |
IRIS_STAGE_DIRTY_CONSTANTS_TCS;
@@ -1525,7 +1527,8 @@ iris_update_compiled_tes(struct iris_context *ice)
shader = iris_compile_tes(ice, ish, &key);
if (old != shader) {
- ice->shaders.prog[IRIS_CACHE_TES] = shader;
+ iris_shader_variant_reference(&ice->shaders.prog[MESA_SHADER_TESS_EVAL],
+ shader);
ice->state.stage_dirty |= IRIS_STAGE_DIRTY_TES |
IRIS_STAGE_DIRTY_BINDINGS_TES |
IRIS_STAGE_DIRTY_CONSTANTS_TES;
@@ -1647,7 +1650,8 @@ iris_update_compiled_gs(struct iris_context *ice)
}
if (old != shader) {
- ice->shaders.prog[IRIS_CACHE_GS] = shader;
+ iris_shader_variant_reference(&ice->shaders.prog[MESA_SHADER_GEOMETRY],
+ shader);
ice->state.stage_dirty |= IRIS_STAGE_DIRTY_GS |
IRIS_STAGE_DIRTY_BINDINGS_GS |
IRIS_STAGE_DIRTY_CONSTANTS_GS;
@@ -1763,7 +1767,8 @@ iris_update_compiled_fs(struct iris_context *ice)
if (old != shader) {
// XXX: only need to flag CLIP if barycentric has NONPERSPECTIVE
// toggles. might be able to avoid flagging SBE too.
- ice->shaders.prog[IRIS_CACHE_FS] = shader;
+ iris_shader_variant_reference(&ice->shaders.prog[MESA_SHADER_FRAGMENT],
+ shader);
ice->state.dirty |= IRIS_DIRTY_WM |
IRIS_DIRTY_CLIP |
IRIS_DIRTY_SBE;
@@ -1858,8 +1863,8 @@ iris_update_compiled_shaders(struct iris_context *ice)
iris_update_compiled_tcs(ice);
iris_update_compiled_tes(ice);
} else {
- ice->shaders.prog[IRIS_CACHE_TCS] = NULL;
- ice->shaders.prog[IRIS_CACHE_TES] = NULL;
+ iris_shader_variant_reference(&ice->shaders.prog[MESA_SHADER_TESS_CTRL], NULL);
+ iris_shader_variant_reference(&ice->shaders.prog[MESA_SHADER_TESS_EVAL], NULL);
ice->state.stage_dirty |=
IRIS_STAGE_DIRTY_TCS | IRIS_STAGE_DIRTY_TES |
IRIS_STAGE_DIRTY_BINDINGS_TCS | IRIS_STAGE_DIRTY_BINDINGS_TES |
@@ -2005,7 +2010,8 @@ iris_update_compiled_cs(struct iris_context *ice)
shader = iris_compile_cs(ice, ish, &key);
if (old != shader) {
- ice->shaders.prog[IRIS_CACHE_CS] = shader;
+ iris_shader_variant_reference(&ice->shaders.prog[MESA_SHADER_COMPUTE],
+ shader);
ice->state.stage_dirty |= IRIS_STAGE_DIRTY_CS |
IRIS_STAGE_DIRTY_BINDINGS_CS |
IRIS_STAGE_DIRTY_CONSTANTS_CS;
@@ -2409,8 +2415,6 @@ iris_delete_shader_state(struct pipe_context *ctx, void *state, gl_shader_stage
ice->state.stage_dirty |= IRIS_STAGE_DIRTY_UNCOMPILED_VS << stage;
}
- iris_delete_shader_variants(ice, ish);
-
ralloc_free(ish->nir);
free(ish);
}
diff --git a/src/gallium/drivers/iris/iris_program_cache.c b/src/gallium/drivers/iris/iris_program_cache.c
index 357c9eec1d3..15f524ff628 100644
--- a/src/gallium/drivers/iris/iris_program_cache.c
+++ b/src/gallium/drivers/iris/iris_program_cache.c
@@ -117,58 +117,12 @@ iris_find_previous_compile(const struct iris_context *ice,
}
void
-iris_delete_shader_variants(struct iris_context *ice,
- struct iris_uncompiled_shader *ish)
+iris_delete_shader_variant(struct iris_compiled_shader *shader)
{
- struct hash_table *cache = ice->shaders.cache;
- gl_shader_stage stage = ish->nir->info.stage;
- enum iris_program_cache_id cache_id = stage;
-
- hash_table_foreach(cache, entry) {
- const struct keybox *keybox = entry->key;
- const struct brw_base_prog_key *key = (const void *)keybox->data;
-
- if (keybox->cache_id == cache_id &&
- key->program_string_id == ish->program_id) {
- struct iris_compiled_shader *shader = entry->data;
-
- _mesa_hash_table_remove(cache, entry);
-
- /* Shader variants may still be bound in the context even after
- * the API-facing shader has been deleted. In particular, a draw
- * may not have triggered iris_update_compiled_shaders() yet. In
- * that case, we may be referring to that shader's VUE map, stream
- * output settings, and so on. We also like to compare the old and
- * new shader programs when swapping them out to flag dirty state.
- *
- * So, it's hazardous to delete a bound shader variant. We avoid
- * doing so, choosing to instead move "deleted" shader variants to
- * a list, deferring the actual deletion until they're not bound.
- *
- * For simplicity, we always move deleted variants to the list,
- * even if we could delete them immediately. We'll then process
- * the list, catching both these variants and any others.
- */
- list_addtail(&shader->link, &ice->shaders.deleted_variants[stage]);
- }
- }
-
- /* Process any pending deferred variant deletions. */
- list_for_each_entry_safe(struct iris_compiled_shader, shader,
- &ice->shaders.deleted_variants[stage], link) {
- /* If the shader is still bound, defer deletion. */
- if (ice->shaders.prog[stage] == shader)
- continue;
-
- list_del(&shader->link);
-
- /* Actually delete the variant. */
- pipe_resource_reference(&shader->assembly.res, NULL);
- ralloc_free(shader);
- }
+ pipe_resource_reference(&shader->assembly.res, NULL);
+ ralloc_free(shader);
}
-
struct iris_compiled_shader *
iris_upload_shader(struct iris_context *ice,
enum iris_program_cache_id cache_id,
@@ -189,6 +143,8 @@ iris_upload_shader(struct iris_context *ice,
rzalloc_size(cache, sizeof(struct iris_compiled_shader) +
screen->vtbl.derived_program_state_size(cache_id));
+ pipe_reference_init(&shader->ref, 1);
+
shader->assembly.res = NULL;
u_upload_alloc(ice->shaders.uploader, 0, prog_data->program_size, 64,
&shader->assembly.offset, &shader->assembly.res,
@@ -213,8 +169,6 @@ iris_upload_shader(struct iris_context *ice,
brw_write_shader_relocs(&screen->devinfo, shader->map, prog_data,
reloc_values, ARRAY_SIZE(reloc_values));
- list_inithead(&shader->link);
-
shader->prog_data = prog_data;
shader->streamout = streamout;
shader->system_values = system_values;
@@ -304,26 +258,18 @@ iris_init_program_cache(struct iris_context *ice)
ice->shaders.uploader =
u_upload_create(&ice->ctx, 16384, PIPE_BIND_CUSTOM, PIPE_USAGE_IMMUTABLE,
IRIS_RESOURCE_FLAG_SHADER_MEMZONE);
-
- for (int i = 0; i < MESA_SHADER_STAGES; i++)
- list_inithead(&ice->shaders.deleted_variants[i]);
}
void
iris_destroy_program_cache(struct iris_context *ice)
{
for (int i = 0; i < MESA_SHADER_STAGES; i++) {
- ice->shaders.prog[i] = NULL;
-
- list_for_each_entry_safe(struct iris_compiled_shader, shader,
- &ice->shaders.deleted_variants[i], link) {
- pipe_resource_reference(&shader->assembly.res, NULL);
- }
+ iris_shader_variant_reference(&ice->shaders.prog[i], NULL);
}
hash_table_foreach(ice->shaders.cache, entry) {
struct iris_compiled_shader *shader = entry->data;
- pipe_resource_reference(&shader->assembly.res, NULL);
+ iris_delete_shader_variant(shader);
}
u_upload_destroy(ice->shaders.uploader);
More information about the mesa-commit
mailing list