spirv: Add better checks for SSA value types
authorJason Ekstrand <jason@jlekstrand.net>
Fri, 29 May 2020 21:10:28 +0000 (16:10 -0500)
committerJason Ekstrand <jason@jlekstrand.net>
Fri, 24 Jul 2020 03:43:21 +0000 (22:43 -0500)
Primarily, we check for two things:

 1. That we only ever add SSA values via vtn_push_ssa_value and
    vtn_copy_value.

 2. That the type of the SSA value matches the SPIR-V destination type.

Reviewed-by: Caio Marcelo de Oliveira Filho <caio.oliveira@intel.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/5278>

src/compiler/spirv/spirv_to_nir.c
src/compiler/spirv/vtn_alu.c
src/compiler/spirv/vtn_private.h
src/compiler/spirv/vtn_variables.c

index 53e082a6bd717468b55879f7b2e9aa96eb5901e2..404fb449bc54f77b155964a4f9bb3954b4ae81e0 100644 (file)
@@ -167,7 +167,7 @@ static struct vtn_ssa_value *
 vtn_undef_ssa_value(struct vtn_builder *b, const struct glsl_type *type)
 {
    struct vtn_ssa_value *val = rzalloc(b, struct vtn_ssa_value);
-   val->type = type;
+   val->type = glsl_get_bare_type(type);
 
    if (glsl_type_is_vector_or_scalar(type)) {
       unsigned num_components = glsl_get_vector_elements(val->type);
@@ -202,7 +202,7 @@ vtn_const_ssa_value(struct vtn_builder *b, nir_constant *constant,
       return entry->data;
 
    struct vtn_ssa_value *val = rzalloc(b, struct vtn_ssa_value);
-   val->type = type;
+   val->type = glsl_get_bare_type(type);
 
    if (glsl_type_is_vector_or_scalar(type)) {
       unsigned num_components = glsl_get_vector_elements(val->type);
@@ -269,11 +269,17 @@ vtn_push_ssa_value(struct vtn_builder *b, uint32_t value_id,
 {
    struct vtn_type *type = vtn_get_value_type(b, value_id);
 
+   /* See vtn_create_ssa_value */
+   vtn_fail_if(ssa->type != glsl_get_bare_type(type->type),
+               "Type mismatch for SPIR-V SSA value");
+
    struct vtn_value *val;
    if (type->base_type == vtn_base_type_pointer) {
       val = vtn_push_pointer(b, value_id, vtn_pointer_from_ssa(b, ssa->def, type));
    } else {
-      val = vtn_push_value(b, value_id, vtn_value_type_ssa);
+      /* Don't trip the value_type_ssa check in vtn_push_value */
+      val = vtn_push_value(b, value_id, vtn_value_type_invalid);
+      val->value_type = vtn_value_type_ssa;
       val->ssa = ssa;
    }
 
@@ -2160,8 +2166,19 @@ vtn_emit_scoped_memory_barrier(struct vtn_builder *b, SpvScope scope,
 struct vtn_ssa_value *
 vtn_create_ssa_value(struct vtn_builder *b, const struct glsl_type *type)
 {
+   /* Always use bare types for SSA values for a couple of reasons:
+    *
+    *  1. Code which emits deref chains should never listen to the explicit
+    *     layout information on the SSA value if any exists.  If we've
+    *     accidentally been relying on this, we want to find those bugs.
+    *
+    *  2. We want to be able to quickly check that an SSA value being assigned
+    *     to a SPIR-V value has the right type.  Using bare types everywhere
+    *     ensures that we can pointer-compare.
+    */
    struct vtn_ssa_value *val = rzalloc(b, struct vtn_ssa_value);
-   val->type = type;
+   val->type = glsl_get_bare_type(type);
+
 
    if (!glsl_type_is_vector_or_scalar(type)) {
       unsigned elems = glsl_get_length(val->type);
index 78ddaf6e6fb410c75f1d9d3d92248cdc3bc1d288..a00e241fe6103ea6c162e440fdd6082b1373da65 100644 (file)
@@ -43,7 +43,7 @@ wrap_matrix(struct vtn_builder *b, struct vtn_ssa_value *val)
       return val;
 
    struct vtn_ssa_value *dest = rzalloc(b, struct vtn_ssa_value);
-   dest->type = val->type;
+   dest->type = glsl_get_bare_type(val->type);
    dest->elems = ralloc_array(b, struct vtn_ssa_value *, 1);
    dest->elems[0] = val;
 
index ea30cb68ddd839002edbea0713c720d5f3aaa9de..278a72e1828f91d8fbe94aeace7f799ade8d71a6 100644 (file)
@@ -710,6 +710,10 @@ vtn_push_value(struct vtn_builder *b, uint32_t value_id,
 {
    struct vtn_value *val = vtn_untyped_value(b, value_id);
 
+   vtn_fail_if(value_type == vtn_value_type_ssa,
+               "Do not call vtn_push_value for value_type_ssa.  Use "
+               "vtn_push_ssa_value instead.");
+
    vtn_fail_if(val->value_type != vtn_value_type_invalid,
                "SPIR-V id %u has already been written by another instruction",
                value_id);
index d2e9f07ebfe06d8c5171d6cc070cced0ffa87d50..60a1672d5dd01698305cf920d04914f9c4bbe27b 100644 (file)
@@ -81,9 +81,13 @@ vtn_copy_value(struct vtn_builder *b, uint32_t src_value_id,
                uint32_t dst_value_id)
 {
    struct vtn_value *src = vtn_untyped_value(b, src_value_id);
-   struct vtn_value *dst = vtn_push_value(b, dst_value_id, src->value_type);
+   struct vtn_value *dst = vtn_untyped_value(b, dst_value_id);
    struct vtn_value src_copy = *src;
 
+   vtn_fail_if(dst->value_type != vtn_value_type_invalid,
+               "SPIR-V id %u has already been written by another instruction",
+               dst_value_id);
+
    vtn_fail_if(dst->type->id != src->type->id,
                "Result Type must equal Operand type");