Skip to content
This repository has been archived by the owner on Jan 7, 2023. It is now read-only.

Commit

Permalink
spirv: Add error checking for Block and BufferBlock decorations
Browse files Browse the repository at this point in the history
Variable pointers being well-defined across the block boundary requires
a couple of very specific SPIR-V validation rules.  Normally, we'd trust
the validator to catch these but since CTS tests have been found in the
wild which violate them, we'll carry our own checks.

Reviewed-by: Alejandro Piñeiro <apinheiro@igalia.com>
Reviewed-by: Caio Marcelo de Oliveira Filho <caio.oliveira@intel.com>
  • Loading branch information
gfxstrand committed Jan 8, 2019
1 parent e90b738 commit 5c3cb9c
Show file tree
Hide file tree
Showing 3 changed files with 53 additions and 0 deletions.
35 changes: 35 additions & 0 deletions src/compiler/spirv/spirv_to_nir.c
Original file line number Diff line number Diff line change
Expand Up @@ -563,6 +563,29 @@ struct member_decoration_ctx {
struct vtn_type *type;
};

/**
* Returns true if the given type contains a struct decorated Block or
* BufferBlock
*/
static bool
vtn_type_contains_block(struct vtn_builder *b, struct vtn_type *type)
{
switch (type->base_type) {
case vtn_base_type_array:
return vtn_type_contains_block(b, type->array_element);
case vtn_base_type_struct:
if (type->block || type->buffer_block)
return true;
for (unsigned i = 0; i < type->length; i++) {
if (vtn_type_contains_block(b, type->members[i]))
return true;
}
return false;
default:
return false;
}
}

/** Returns true if two types are "compatible", i.e. you can do an OpLoad,
* OpStore, or OpCopyMemory between them without breaking anything.
* Technically, the SPIR-V rules require the exact same type ID but this lets
Expand Down Expand Up @@ -1375,6 +1398,17 @@ vtn_handle_type(struct vtn_builder *b, SpvOp opcode,
}

vtn_foreach_decoration(b, val, type_decoration_cb, NULL);

if (val->type->base_type == vtn_base_type_struct &&
(val->type->block || val->type->buffer_block)) {
for (unsigned i = 0; i < val->type->length; i++) {
vtn_fail_if(vtn_type_contains_block(b, val->type->members[i]),
"Block and BufferBlock decorations cannot decorate a "
"structure type that is nested at any level inside "
"another structure type decorated with Block or "
"BufferBlock.");
}
}
}

static nir_constant *
Expand Down Expand Up @@ -3598,6 +3632,7 @@ vtn_handle_preamble_instruction(struct vtn_builder *b, SpvOp opcode,
case SpvCapabilityVariablePointersStorageBuffer:
case SpvCapabilityVariablePointers:
spv_check_supported(variable_pointers, cap);
b->variable_pointers = true;
break;

case SpvCapabilityStorageUniformBufferBlock16:
Expand Down
1 change: 1 addition & 0 deletions src/compiler/spirv/vtn_private.h
Original file line number Diff line number Diff line change
Expand Up @@ -586,6 +586,7 @@ struct vtn_builder {
struct vtn_value *entry_point;
bool origin_upper_left;
bool pixel_center_integer;
bool variable_pointers;

struct vtn_function *func;
struct exec_list functions;
Expand Down
17 changes: 17 additions & 0 deletions src/compiler/spirv/vtn_variables.c
Original file line number Diff line number Diff line change
Expand Up @@ -1686,9 +1686,26 @@ vtn_create_variable(struct vtn_builder *b, struct vtn_value *val,

switch (mode) {
case vtn_variable_mode_ubo:
/* There's no other way to get vtn_variable_mode_ubo */
vtn_assert(without_array->block);
b->shader->info.num_ubos++;
break;
case vtn_variable_mode_ssbo:
if (storage_class == SpvStorageClassStorageBuffer &&
!without_array->block) {
if (b->variable_pointers) {
vtn_fail("Variables in the StorageBuffer storage class must "
"have a struct type with the Block decoration");
} else {
/* If variable pointers are not present, it's still malformed
* SPIR-V but we can parse it and do the right thing anyway.
* Since some of the 8-bit storage tests have bugs in this are,
* just make it a warning for now.
*/
vtn_warn("Variables in the StorageBuffer storage class must "
"have a struct type with the Block decoration");
}
}
b->shader->info.num_ssbos++;
break;
case vtn_variable_mode_uniform:
Expand Down

0 comments on commit 5c3cb9c

Please sign in to comment.