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 <pierre-eric.pelloux-prayer@amd.com>
Tested-by: Marge Bot <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/4078>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/4078>

diff --git a/src/gallium/auxiliary/cso_cache/cso_context.c b/src/gallium/auxiliary/cso_cache/cso_context.c
index ff40f19e68514205b2457e8498da24ab5b78d1f3..5c21f8279af47b286e05b6056a27833093fc8f8d 100644 (file)
--- 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 447630943f5499d7b9d95cafe5a0b9ccca7417de..0cd977b39e040c2e142f9521f5370f41b1afdaa1 100644 (file)
--- 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 04c50ae619ff5e64708a5896c61fce444c3bea55..600e298a3933d3cb7710160a808bbfdd10f686c3 100644 (file)
--- 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 caa300461cdcf5e2c3cb38c35184b2b9bd8d62a8..83f6db128c3de852a71abd4f634d4f806fdf24dd 100644 (file)
--- 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 436f7019ae2fc56fe9131baea2da2a801d862103..3cf4c79f083a573b0d15822e03c8b86c2693e534 100644 (file)
--- 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 5fb2fba2f24ee3b10bb1c3a065fbfe2fb6b32ca8..6e1b2f11f73a857452a3ba1a9c38b48a43f8b4ac 100644 (file)
--- 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 ebcfe1973fc88deddfb9ce9b84b88f7087c4919e..5982904d3ba9b2d2f60cd1dc62ae462084e1c222 100644 (file)
--- 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 baf354639e22baaede9b2cd95b4cd298188bfa53..8bb94bac0531d0ca4aaa83a33f8b8b574dbd241d 100644 (file)
--- 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 bbb348f02002c7f44b75f683083fd4e3b9498dbb..ee5ee2df59868da854e9ddf9ac511b4e68ff0af8 100644 (file)
--- 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 bbf639ac1800a547dc9397a5d85fe9119b04fa3b..dba4795e6d7852eb131b3b7c47875cdffcdacf5b 100644 (file)
--- 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 */