Module: Mesa
Branch: master
Commit: 2dc300421d3079d653f106a876263904ba0faacc
URL:    
http://cgit.freedesktop.org/mesa/mesa/commit/?id=2dc300421d3079d653f106a876263904ba0faacc

Author: Marek Olšák <marek.ol...@amd.com>
Date:   Thu Mar  5 15:09:28 2020 -0500

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-pra...@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>

---

 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 ff40f19e685..5c21f8279af 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 447630943f5..0cd977b39e0 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 04c50ae619f..600e298a393 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 caa300461cd..83f6db128c3 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 436f7019ae2..3cf4c79f083 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 5fb2fba2f24..6e1b2f11f73 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 ebcfe1973fc..5982904d3ba 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 baf354639e2..8bb94bac053 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 bbb348f0200..ee5ee2df598 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 bbf639ac180..dba4795e6d7 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 */

_______________________________________________
mesa-commit mailing list
mesa-commit@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-commit

Reply via email to