From d0be2fed4edaa5f00433f41a0f4c660330348191 Mon Sep 17 00:00:00 2001 From: Jason Ekstrand Date: Fri, 29 May 2020 16:10:28 -0500 Subject: [PATCH] spirv: Add better checks for SSA value types 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 Part-of: --- src/compiler/spirv/spirv_to_nir.c | 25 +++++++++++++++++++++---- src/compiler/spirv/vtn_alu.c | 2 +- src/compiler/spirv/vtn_private.h | 4 ++++ src/compiler/spirv/vtn_variables.c | 6 +++++- 4 files changed, 31 insertions(+), 6 deletions(-) diff --git a/src/compiler/spirv/spirv_to_nir.c b/src/compiler/spirv/spirv_to_nir.c index 53e082a6bd7..404fb449bc5 100644 --- a/src/compiler/spirv/spirv_to_nir.c +++ b/src/compiler/spirv/spirv_to_nir.c @@ -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); diff --git a/src/compiler/spirv/vtn_alu.c b/src/compiler/spirv/vtn_alu.c index 78ddaf6e6fb..a00e241fe61 100644 --- a/src/compiler/spirv/vtn_alu.c +++ b/src/compiler/spirv/vtn_alu.c @@ -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; diff --git a/src/compiler/spirv/vtn_private.h b/src/compiler/spirv/vtn_private.h index ea30cb68ddd..278a72e1828 100644 --- a/src/compiler/spirv/vtn_private.h +++ b/src/compiler/spirv/vtn_private.h @@ -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); diff --git a/src/compiler/spirv/vtn_variables.c b/src/compiler/spirv/vtn_variables.c index d2e9f07ebfe..60a1672d5dd 100644 --- a/src/compiler/spirv/vtn_variables.c +++ b/src/compiler/spirv/vtn_variables.c @@ -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"); -- 2.30.2