From 2dc300421d3079d653f106a876263904ba0faacc Mon Sep 17 00:00:00 2001 From: =?utf8?q?Marek=20Ol=C5=A1=C3=A1k?= Date: Thu, 5 Mar 2020 15:09:28 -0500 Subject: [PATCH] gallium/cso_context: remove cso_delete_xxx_shader helpers to fix the live cache With the live shader cache, equivalent shaders can be backed by the same CSO. This breaks the logic that identifies whether the shader being deleted is bound. For example, having shaders A and B, you can bind shader A and delete shader B. Deleting shader B will unbind shader A if they are equivalent. Pierre-Eric figured out the root cause for this issue. Fixes: 0db74f479b9 - radeonsi: use the live shader cache Closes: #2596 Acked-by: Pierre-Eric Pelloux-Prayer Tested-by: Marge Bot Part-of: --- src/gallium/auxiliary/cso_cache/cso_context.c | 60 ------------------------- src/gallium/auxiliary/cso_cache/cso_context.h | 16 ------- src/gallium/state_trackers/nine/vertexshader9.c | 2 +- src/gallium/state_trackers/xa/xa_tgsi.c | 10 ++--- src/mesa/state_tracker/st_cb_clear.c | 8 ++-- src/mesa/state_tracker/st_cb_drawpixels.c | 5 +-- src/mesa/state_tracker/st_cb_drawtex.c | 2 +- src/mesa/state_tracker/st_context.c | 12 ++--- src/mesa/state_tracker/st_pbo.c | 8 ++-- src/mesa/state_tracker/st_program.c | 57 ++++++++++++++++++++--- 10 files changed, 74 insertions(+), 106 deletions(-) diff --git a/src/gallium/auxiliary/cso_cache/cso_context.c b/src/gallium/auxiliary/cso_cache/cso_context.c index ff40f19..5c21f82 100644 --- a/src/gallium/auxiliary/cso_cache/cso_context.c +++ b/src/gallium/auxiliary/cso_cache/cso_context.c @@ -662,16 +662,6 @@ void cso_set_fragment_shader_handle(struct cso_context *ctx, void *handle ) } } -void cso_delete_fragment_shader(struct cso_context *ctx, void *handle ) -{ - if (handle == ctx->fragment_shader) { - /* unbind before deleting */ - ctx->pipe->bind_fs_state(ctx->pipe, NULL); - ctx->fragment_shader = NULL; - } - ctx->pipe->delete_fs_state(ctx->pipe, handle); -} - static void cso_save_fragment_shader(struct cso_context *ctx) { @@ -698,16 +688,6 @@ void cso_set_vertex_shader_handle(struct cso_context *ctx, void *handle) } } -void cso_delete_vertex_shader(struct cso_context *ctx, void *handle ) -{ - if (handle == ctx->vertex_shader) { - /* unbind before deleting */ - ctx->pipe->bind_vs_state(ctx->pipe, NULL); - ctx->vertex_shader = NULL; - } - ctx->pipe->delete_vs_state(ctx->pipe, handle); -} - static void cso_save_vertex_shader(struct cso_context *ctx) { @@ -914,16 +894,6 @@ void cso_set_geometry_shader_handle(struct cso_context *ctx, void *handle) } } -void cso_delete_geometry_shader(struct cso_context *ctx, void *handle) -{ - if (handle == ctx->geometry_shader) { - /* unbind before deleting */ - ctx->pipe->bind_gs_state(ctx->pipe, NULL); - ctx->geometry_shader = NULL; - } - ctx->pipe->delete_gs_state(ctx->pipe, handle); -} - static void cso_save_geometry_shader(struct cso_context *ctx) { @@ -959,16 +929,6 @@ void cso_set_tessctrl_shader_handle(struct cso_context *ctx, void *handle) } } -void cso_delete_tessctrl_shader(struct cso_context *ctx, void *handle) -{ - if (handle == ctx->tessctrl_shader) { - /* unbind before deleting */ - ctx->pipe->bind_tcs_state(ctx->pipe, NULL); - ctx->tessctrl_shader = NULL; - } - ctx->pipe->delete_tcs_state(ctx->pipe, handle); -} - static void cso_save_tessctrl_shader(struct cso_context *ctx) { @@ -1004,16 +964,6 @@ void cso_set_tesseval_shader_handle(struct cso_context *ctx, void *handle) } } -void cso_delete_tesseval_shader(struct cso_context *ctx, void *handle) -{ - if (handle == ctx->tesseval_shader) { - /* unbind before deleting */ - ctx->pipe->bind_tes_state(ctx->pipe, NULL); - ctx->tesseval_shader = NULL; - } - ctx->pipe->delete_tes_state(ctx->pipe, handle); -} - static void cso_save_tesseval_shader(struct cso_context *ctx) { @@ -1049,16 +999,6 @@ void cso_set_compute_shader_handle(struct cso_context *ctx, void *handle) } } -void cso_delete_compute_shader(struct cso_context *ctx, void *handle) -{ - if (handle == ctx->compute_shader) { - /* unbind before deleting */ - ctx->pipe->bind_compute_state(ctx->pipe, NULL); - ctx->compute_shader = NULL; - } - ctx->pipe->delete_compute_state(ctx->pipe, handle); -} - static void cso_set_vertex_elements_direct(struct cso_context *ctx, const struct cso_velems_state *velems) diff --git a/src/gallium/auxiliary/cso_cache/cso_context.h b/src/gallium/auxiliary/cso_cache/cso_context.h index 4476309..0cd977b 100644 --- a/src/gallium/auxiliary/cso_cache/cso_context.h +++ b/src/gallium/auxiliary/cso_cache/cso_context.h @@ -103,27 +103,11 @@ void cso_set_stream_outputs(struct cso_context *ctx, */ void cso_set_fragment_shader_handle(struct cso_context *ctx, void *handle); -void cso_delete_fragment_shader(struct cso_context *ctx, void *handle ); - - void cso_set_vertex_shader_handle(struct cso_context *ctx, void *handle); -void cso_delete_vertex_shader(struct cso_context *ctx, void *handle ); - - void cso_set_geometry_shader_handle(struct cso_context *ctx, void *handle); -void cso_delete_geometry_shader(struct cso_context *ctx, void *handle); - - void cso_set_tessctrl_shader_handle(struct cso_context *ctx, void *handle); -void cso_delete_tessctrl_shader(struct cso_context *ctx, void *handle); - - void cso_set_tesseval_shader_handle(struct cso_context *ctx, void *handle); -void cso_delete_tesseval_shader(struct cso_context *ctx, void *handle); - - void cso_set_compute_shader_handle(struct cso_context *ctx, void *handle); -void cso_delete_compute_shader(struct cso_context *ctx, void *handle); void cso_set_framebuffer(struct cso_context *cso, diff --git a/src/gallium/state_trackers/nine/vertexshader9.c b/src/gallium/state_trackers/nine/vertexshader9.c index 04c50ae..600e298 100644 --- a/src/gallium/state_trackers/nine/vertexshader9.c +++ b/src/gallium/state_trackers/nine/vertexshader9.c @@ -143,7 +143,7 @@ NineVertexShader9_dtor( struct NineVertexShader9 *This ) while (var_so && var_so->vdecl) { if (var_so->cso) { - cso_delete_vertex_shader(This->base.device->cso_sw, var_so->cso ); + This->base.device->pipe_sw->delete_vs_state(This->base.device->pipe_sw, var_so->cso); } var_so = var_so->next; } diff --git a/src/gallium/state_trackers/xa/xa_tgsi.c b/src/gallium/state_trackers/xa/xa_tgsi.c index caa3004..83f6db1 100644 --- a/src/gallium/state_trackers/xa/xa_tgsi.c +++ b/src/gallium/state_trackers/xa/xa_tgsi.c @@ -431,7 +431,7 @@ xa_shaders_create(struct xa_context *r) } static void -cache_destroy(struct cso_context *cso, +cache_destroy(struct pipe_context *pipe, struct cso_hash *hash, unsigned processor) { struct cso_hash_iter iter = cso_hash_first_node(hash); @@ -440,9 +440,9 @@ cache_destroy(struct cso_context *cso, void *shader = (void *)cso_hash_iter_data(iter); if (processor == PIPE_SHADER_FRAGMENT) { - cso_delete_fragment_shader(cso, shader); + pipe->delete_fs_state(pipe, shader); } else if (processor == PIPE_SHADER_VERTEX) { - cso_delete_vertex_shader(cso, shader); + pipe->delete_vs_state(pipe, shader); } iter = cso_hash_erase(hash, iter); } @@ -452,8 +452,8 @@ cache_destroy(struct cso_context *cso, void xa_shaders_destroy(struct xa_shaders *sc) { - cache_destroy(sc->r->cso, &sc->vs_hash, PIPE_SHADER_VERTEX); - cache_destroy(sc->r->cso, &sc->fs_hash, PIPE_SHADER_FRAGMENT); + cache_destroy(sc->r->pipe, &sc->vs_hash, PIPE_SHADER_VERTEX); + cache_destroy(sc->r->pipe, &sc->fs_hash, PIPE_SHADER_FRAGMENT); FREE(sc); } diff --git a/src/mesa/state_tracker/st_cb_clear.c b/src/mesa/state_tracker/st_cb_clear.c index 436f701..3cf4c79 100644 --- a/src/mesa/state_tracker/st_cb_clear.c +++ b/src/mesa/state_tracker/st_cb_clear.c @@ -85,19 +85,19 @@ void st_destroy_clear(struct st_context *st) { if (st->clear.fs) { - cso_delete_fragment_shader(st->cso_context, st->clear.fs); + st->pipe->delete_fs_state(st->pipe, st->clear.fs); st->clear.fs = NULL; } if (st->clear.vs) { - cso_delete_vertex_shader(st->cso_context, st->clear.vs); + st->pipe->delete_vs_state(st->pipe, st->clear.vs); st->clear.vs = NULL; } if (st->clear.vs_layered) { - cso_delete_vertex_shader(st->cso_context, st->clear.vs_layered); + st->pipe->delete_vs_state(st->pipe, st->clear.vs_layered); st->clear.vs_layered = NULL; } if (st->clear.gs_layered) { - cso_delete_geometry_shader(st->cso_context, st->clear.gs_layered); + st->pipe->delete_gs_state(st->pipe, st->clear.gs_layered); st->clear.gs_layered = NULL; } } diff --git a/src/mesa/state_tracker/st_cb_drawpixels.c b/src/mesa/state_tracker/st_cb_drawpixels.c index 5fb2fba..6e1b2f1 100644 --- a/src/mesa/state_tracker/st_cb_drawpixels.c +++ b/src/mesa/state_tracker/st_cb_drawpixels.c @@ -1916,12 +1916,11 @@ st_destroy_drawpix(struct st_context *st) for (i = 0; i < ARRAY_SIZE(st->drawpix.zs_shaders); i++) { if (st->drawpix.zs_shaders[i]) - cso_delete_fragment_shader(st->cso_context, - st->drawpix.zs_shaders[i]); + st->pipe->delete_fs_state(st->pipe, st->drawpix.zs_shaders[i]); } if (st->passthrough_vs) - cso_delete_vertex_shader(st->cso_context, st->passthrough_vs); + st->pipe->delete_vs_state(st->pipe, st->passthrough_vs); /* Free cache data */ for (i = 0; i < ARRAY_SIZE(st->drawpix_cache.entries); i++) { diff --git a/src/mesa/state_tracker/st_cb_drawtex.c b/src/mesa/state_tracker/st_cb_drawtex.c index ebcfe19..5982904 100644 --- a/src/mesa/state_tracker/st_cb_drawtex.c +++ b/src/mesa/state_tracker/st_cb_drawtex.c @@ -362,7 +362,7 @@ st_destroy_drawtex(struct st_context *st) { GLuint i; for (i = 0; i < NumCachedShaders; i++) { - cso_delete_vertex_shader(st->cso_context, CachedShaders[i].handle); + st->pipe->delete_vs_state(st->pipe, CachedShaders[i].handle); } NumCachedShaders = 0; } diff --git a/src/mesa/state_tracker/st_context.c b/src/mesa/state_tracker/st_context.c index baf3546..8bb94ba 100644 --- a/src/mesa/state_tracker/st_context.c +++ b/src/mesa/state_tracker/st_context.c @@ -399,22 +399,22 @@ free_zombie_shaders(struct st_context *st) switch (entry->type) { case PIPE_SHADER_VERTEX: - cso_delete_vertex_shader(st->cso_context, entry->shader); + st->pipe->delete_vs_state(st->pipe, entry->shader); break; case PIPE_SHADER_FRAGMENT: - cso_delete_fragment_shader(st->cso_context, entry->shader); + st->pipe->delete_fs_state(st->pipe, entry->shader); break; case PIPE_SHADER_GEOMETRY: - cso_delete_geometry_shader(st->cso_context, entry->shader); + st->pipe->delete_gs_state(st->pipe, entry->shader); break; case PIPE_SHADER_TESS_CTRL: - cso_delete_tessctrl_shader(st->cso_context, entry->shader); + st->pipe->delete_tcs_state(st->pipe, entry->shader); break; case PIPE_SHADER_TESS_EVAL: - cso_delete_tesseval_shader(st->cso_context, entry->shader); + st->pipe->delete_tes_state(st->pipe, entry->shader); break; case PIPE_SHADER_COMPUTE: - cso_delete_compute_shader(st->cso_context, entry->shader); + st->pipe->delete_compute_state(st->pipe, entry->shader); break; default: unreachable("invalid shader type in free_zombie_shaders()"); diff --git a/src/mesa/state_tracker/st_pbo.c b/src/mesa/state_tracker/st_pbo.c index bbb348f..ee5ee2d 100644 --- a/src/mesa/state_tracker/st_pbo.c +++ b/src/mesa/state_tracker/st_pbo.c @@ -830,7 +830,7 @@ st_destroy_pbo_helpers(struct st_context *st) for (i = 0; i < ARRAY_SIZE(st->pbo.upload_fs); ++i) { if (st->pbo.upload_fs[i]) { - cso_delete_fragment_shader(st->cso_context, st->pbo.upload_fs[i]); + st->pipe->delete_fs_state(st->pipe, st->pbo.upload_fs[i]); st->pbo.upload_fs[i] = NULL; } } @@ -838,19 +838,19 @@ st_destroy_pbo_helpers(struct st_context *st) for (i = 0; i < ARRAY_SIZE(st->pbo.download_fs); ++i) { for (unsigned j = 0; j < ARRAY_SIZE(st->pbo.download_fs[0]); ++j) { if (st->pbo.download_fs[i][j]) { - cso_delete_fragment_shader(st->cso_context, st->pbo.download_fs[i][j]); + st->pipe->delete_fs_state(st->pipe, st->pbo.download_fs[i][j]); st->pbo.download_fs[i][j] = NULL; } } } if (st->pbo.gs) { - cso_delete_geometry_shader(st->cso_context, st->pbo.gs); + st->pipe->delete_gs_state(st->pipe, st->pbo.gs); st->pbo.gs = NULL; } if (st->pbo.vs) { - cso_delete_vertex_shader(st->cso_context, st->pbo.vs); + st->pipe->delete_vs_state(st->pipe, st->pbo.vs); st->pbo.vs = NULL; } } diff --git a/src/mesa/state_tracker/st_program.c b/src/mesa/state_tracker/st_program.c index bbf639a..dba4795 100644 --- a/src/mesa/state_tracker/st_program.c +++ b/src/mesa/state_tracker/st_program.c @@ -228,22 +228,22 @@ delete_variant(struct st_context *st, struct st_variant *v, GLenum target) */ switch (target) { case GL_VERTEX_PROGRAM_ARB: - cso_delete_vertex_shader(st->cso_context, v->driver_shader); + st->pipe->delete_vs_state(st->pipe, v->driver_shader); break; case GL_TESS_CONTROL_PROGRAM_NV: - cso_delete_tessctrl_shader(st->cso_context, v->driver_shader); + st->pipe->delete_tcs_state(st->pipe, v->driver_shader); break; case GL_TESS_EVALUATION_PROGRAM_NV: - cso_delete_tesseval_shader(st->cso_context, v->driver_shader); + st->pipe->delete_tes_state(st->pipe, v->driver_shader); break; case GL_GEOMETRY_PROGRAM_NV: - cso_delete_geometry_shader(st->cso_context, v->driver_shader); + st->pipe->delete_gs_state(st->pipe, v->driver_shader); break; case GL_FRAGMENT_PROGRAM_ARB: - cso_delete_fragment_shader(st->cso_context, v->driver_shader); + st->pipe->delete_fs_state(st->pipe, v->driver_shader); break; case GL_COMPUTE_PROGRAM_NV: - cso_delete_compute_shader(st->cso_context, v->driver_shader); + st->pipe->delete_compute_state(st->pipe, v->driver_shader); break; default: unreachable("bad shader type in delete_basic_variant"); @@ -262,6 +262,39 @@ delete_variant(struct st_context *st, struct st_variant *v, GLenum target) free(v); } +static void +st_unbind_program(struct st_context *st, struct st_program *p) +{ + /* Unbind the shader in cso_context and re-bind in st/mesa. */ + switch (p->Base.info.stage) { + case MESA_SHADER_VERTEX: + cso_set_vertex_shader_handle(st->cso_context, NULL); + st->dirty |= ST_NEW_VS_STATE; + break; + case MESA_SHADER_TESS_CTRL: + cso_set_tessctrl_shader_handle(st->cso_context, NULL); + st->dirty |= ST_NEW_TCS_STATE; + break; + case MESA_SHADER_TESS_EVAL: + cso_set_tesseval_shader_handle(st->cso_context, NULL); + st->dirty |= ST_NEW_TES_STATE; + break; + case MESA_SHADER_GEOMETRY: + cso_set_geometry_shader_handle(st->cso_context, NULL); + st->dirty |= ST_NEW_GS_STATE; + break; + case MESA_SHADER_FRAGMENT: + cso_set_fragment_shader_handle(st->cso_context, NULL); + st->dirty |= ST_NEW_FS_STATE; + break; + case MESA_SHADER_COMPUTE: + cso_set_compute_shader_handle(st->cso_context, NULL); + st->dirty |= ST_NEW_CS_STATE; + break; + default: + unreachable("invalid shader type"); + } +} /** * Free all basic program variants. @@ -271,6 +304,12 @@ st_release_variants(struct st_context *st, struct st_program *p) { struct st_variant *v; + /* If we are releasing shaders, re-bind them, because we don't + * know which shaders are bound in the driver. + */ + if (p->variants) + st_unbind_program(st, p); + for (v = p->variants; v; ) { struct st_variant *next = v->next; delete_variant(st, v, p->Base.Target); @@ -1770,10 +1809,16 @@ destroy_program_variants(struct st_context *st, struct gl_program *target) struct st_program *p = st_program(target); struct st_variant *v, **prevPtr = &p->variants; + bool unbound = false; for (v = p->variants; v; ) { struct st_variant *next = v->next; if (v->st == st) { + if (!unbound) { + st_unbind_program(st, p); + unbound = true; + } + /* unlink from list */ *prevPtr = next; /* destroy this variant */ -- 2.7.4