Mesa (main): v3dv/pipeline: include pipeline layout on the pipeline sha1

[GitLab Mirror]

Module: Mesa
Branch: main
Commit: 1ed2b5e253146f54ae35d178539b5fd8ff92257f
URL:    http://cgit.freedesktop.org/mesa/mesa/commit/?id=1ed2b5e253146f54ae35d178539b5fd8ff92257f

Author: Alejandro Piñeiro <apinheiro at igalia.com>
Date:   Tue May  3 12:48:10 2022 +0200

v3dv/pipeline: include pipeline layout on the pipeline sha1

Fixes failures on tests like this when the on-disk-cache is enabled:
dEQP-VK.binding_model.descriptor_copy.compute.uniform_buffer_0

We only found them when running full CTS runs. What happens is that we
got a hit from the on-disk shader cache, for several tests using the
same shaders. But some tests seems to be using a uniform buffer, and
others a inline buffer. Right now inline buffers leads to some changes
on the final nir shader, and generated assembly, compared with uniform
buffers. So we got a wrong shader. Fortunately we only got an assert
instead of weird behaviour.

With this commit we include the pipeline layout on the pipeline sha1,
so those two cases would get different sha1. FWIW, this is what other
drivers are already doing.

Surprisingly that didn't cause a problem before.

Reviewed-by: Juan A. Suarez <jasuarez at igalia.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/16313>

---

 src/broadcom/vulkan/v3dv_descriptor_set.c | 40 +++++++++++++++++++++++++++++++
 src/broadcom/vulkan/v3dv_pipeline.c       | 10 ++++++++
 src/broadcom/vulkan/v3dv_private.h        |  4 +++-
 3 files changed, 53 insertions(+), 1 deletion(-)

diff --git a/src/broadcom/vulkan/v3dv_descriptor_set.c b/src/broadcom/vulkan/v3dv_descriptor_set.c
index 653f5d39d05..9bd50cccb2c 100644
--- a/src/broadcom/vulkan/v3dv_descriptor_set.c
+++ b/src/broadcom/vulkan/v3dv_descriptor_set.c
@@ -284,6 +284,36 @@ v3dv_descriptor_map_get_texture_shader_state(struct v3dv_device *device,
    return reloc;
 }
 
+#define SHA1_UPDATE_VALUE(ctx, x) _mesa_sha1_update(ctx, &(x), sizeof(x));
+
+static void
+sha1_update_descriptor_set_binding_layout(struct mesa_sha1 *ctx,
+                                          const struct v3dv_descriptor_set_binding_layout *layout)
+{
+   SHA1_UPDATE_VALUE(ctx, layout->type);
+   SHA1_UPDATE_VALUE(ctx, layout->array_size);
+   SHA1_UPDATE_VALUE(ctx, layout->descriptor_index);
+   SHA1_UPDATE_VALUE(ctx, layout->dynamic_offset_count);
+   SHA1_UPDATE_VALUE(ctx, layout->dynamic_offset_index);
+   SHA1_UPDATE_VALUE(ctx, layout->descriptor_offset);
+   SHA1_UPDATE_VALUE(ctx, layout->immutable_samplers_offset);
+}
+
+static void
+sha1_update_descriptor_set_layout(struct mesa_sha1 *ctx,
+                                  const struct v3dv_descriptor_set_layout *layout)
+{
+   SHA1_UPDATE_VALUE(ctx, layout->flags);
+   SHA1_UPDATE_VALUE(ctx, layout->binding_count);
+   SHA1_UPDATE_VALUE(ctx, layout->shader_stages);
+   SHA1_UPDATE_VALUE(ctx, layout->descriptor_count);
+   SHA1_UPDATE_VALUE(ctx, layout->dynamic_offset_count);
+
+   for (uint16_t i = 0; i < layout->binding_count; i++)
+      sha1_update_descriptor_set_binding_layout(ctx, &layout->binding[i]);
+}
+
+
 /*
  * As anv and tu already points:
  *
@@ -336,6 +366,16 @@ v3dv_CreatePipelineLayout(VkDevice _device,
 
    layout->dynamic_offset_count = dynamic_offset_count;
 
+   struct mesa_sha1 ctx;
+   _mesa_sha1_init(&ctx);
+   for (unsigned s = 0; s < layout->num_sets; s++) {
+      sha1_update_descriptor_set_layout(&ctx, layout->set[s].layout);
+      _mesa_sha1_update(&ctx, &layout->set[s].dynamic_offset_start,
+                        sizeof(layout->set[s].dynamic_offset_start));
+   }
+   _mesa_sha1_update(&ctx, &layout->num_sets, sizeof(layout->num_sets));
+   _mesa_sha1_final(&ctx, layout->sha1);
+
    *pPipelineLayout = v3dv_pipeline_layout_to_handle(layout);
 
    return VK_SUCCESS;
diff --git a/src/broadcom/vulkan/v3dv_pipeline.c b/src/broadcom/vulkan/v3dv_pipeline.c
index d2ae7cbec38..66ceb12f761 100644
--- a/src/broadcom/vulkan/v3dv_pipeline.c
+++ b/src/broadcom/vulkan/v3dv_pipeline.c
@@ -1479,6 +1479,11 @@ pipeline_hash_graphics(const struct v3dv_pipeline *pipeline,
    struct mesa_sha1 ctx;
    _mesa_sha1_init(&ctx);
 
+   if (pipeline->layout) {
+      _mesa_sha1_update(&ctx, &pipeline->layout->sha1,
+                        sizeof(pipeline->layout->sha1));
+   }
+
    /* We need to include all shader stages in the sha1 key as linking may modify
     * the shader code in any stage. An alternative would be to use the
     * serialized NIR, but that seems like an overkill.
@@ -1507,6 +1512,11 @@ pipeline_hash_compute(const struct v3dv_pipeline *pipeline,
    struct mesa_sha1 ctx;
    _mesa_sha1_init(&ctx);
 
+   if (pipeline->layout) {
+      _mesa_sha1_update(&ctx, &pipeline->layout->sha1,
+                        sizeof(pipeline->layout->sha1));
+   }
+
    _mesa_sha1_update(&ctx, pipeline->cs->shader_sha1,
                      sizeof(pipeline->cs->shader_sha1));
 
diff --git a/src/broadcom/vulkan/v3dv_private.h b/src/broadcom/vulkan/v3dv_private.h
index 411047b50de..58d690eee25 100644
--- a/src/broadcom/vulkan/v3dv_private.h
+++ b/src/broadcom/vulkan/v3dv_private.h
@@ -1692,6 +1692,8 @@ struct v3dv_pipeline_layout {
 
    uint32_t dynamic_offset_count;
    uint32_t push_constant_size;
+
+   unsigned char sha1[20];
 };
 
 /*
@@ -1906,7 +1908,7 @@ struct v3dv_pipeline {
 
    struct v3dv_pipeline_shared_data *shared_data;
 
-   /* It is the combined stages sha1, plus the pipeline key sha1. */
+   /* It is the combined stages sha1, layout sha1, plus the pipeline key sha1. */
    unsigned char sha1[20];
 
    /* In general we can reuse v3dv_device->default_attribute_float, so note