From 386f09be9b7984d99d7106b2ec10ba25d5cf2673 Mon Sep 17 00:00:00 2001 From: Neil Roberts Date: Thu, 1 Mar 2018 17:51:58 +0100 Subject: [PATCH] spirv: Get rid of vtn_variable_mode_image/sampler MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit vtn_variable_mode_image and _sampler are instead replaced with vtn_variable_mode_uniform which encompasses both of them. In the few places where it was neccessary to distinguish between the two, the GLSL type of the pointer is used instead. The main reason to do this is that on OpenGL it is permitted to put images and samplers into structs and declare a uniform with them. That means that variables can now have a mix of uniform, sampler and image modes so picking a single one of those modes for a variable no longer makes sense. This fixes OpLoad on a sampler within a struct which was previously using the variable mode to determine whether it was a sampler or not. The type of the variable is a struct so it was not being considered to be uniform mode even though the member being loaded should be sampler mode. The previous code appeared to be using var->interface_type as a place to store the type of the variable without the enclosing array for images and samplers. I guess this worked because opaque types can not appear in interfaces so the interface_type is sort of unused. This patch removes the overloading of var->interface_type and any places that needed the type without the array can now just deduce it from var->type. v2: squash in this patch the changes to anv/nir (Timothy) Signed-off-by: Eduardo Lima Signed-off-by: Neil Roberts Reviewed-by: Timothy Arceri --- src/compiler/spirv/spirv_to_nir.c | 4 +- src/compiler/spirv/vtn_cfg.c | 4 +- src/compiler/spirv/vtn_private.h | 2 - src/compiler/spirv/vtn_variables.c | 43 ++++++------------- .../vulkan/anv_nir_apply_pipeline_layout.c | 6 ++- 5 files changed, 20 insertions(+), 39 deletions(-) diff --git a/src/compiler/spirv/spirv_to_nir.c b/src/compiler/spirv/spirv_to_nir.c index 363be1c4a05..badbae10bfd 100644 --- a/src/compiler/spirv/spirv_to_nir.c +++ b/src/compiler/spirv/spirv_to_nir.c @@ -3742,10 +3742,10 @@ vtn_handle_body_instruction(struct vtn_builder *b, SpvOp opcode, case SpvOpImageQuerySize: { struct vtn_pointer *image = vtn_value(b, w[3], vtn_value_type_pointer)->pointer; - if (image->mode == vtn_variable_mode_image) { + if (glsl_type_is_image(image->type->type)) { vtn_handle_image(b, opcode, w, count); } else { - vtn_assert(image->mode == vtn_variable_mode_sampler); + vtn_assert(glsl_type_is_sampler(image->type->type)); vtn_handle_texture(b, opcode, w, count); } break; diff --git a/src/compiler/spirv/vtn_cfg.c b/src/compiler/spirv/vtn_cfg.c index ad4374112e1..cea40cefc85 100644 --- a/src/compiler/spirv/vtn_cfg.c +++ b/src/compiler/spirv/vtn_cfg.c @@ -124,10 +124,10 @@ vtn_cfg_handle_prepass_instruction(struct vtn_builder *b, SpvOp opcode, without_array = without_array->array_element; if (glsl_type_is_image(without_array->type)) { - vtn_var->mode = vtn_variable_mode_image; + vtn_var->mode = vtn_variable_mode_uniform; param->interface_type = without_array->type; } else if (glsl_type_is_sampler(without_array->type)) { - vtn_var->mode = vtn_variable_mode_sampler; + vtn_var->mode = vtn_variable_mode_uniform; param->interface_type = without_array->type; } else { vtn_var->mode = vtn_variable_mode_param; diff --git a/src/compiler/spirv/vtn_private.h b/src/compiler/spirv/vtn_private.h index 98b326a8830..232396775df 100644 --- a/src/compiler/spirv/vtn_private.h +++ b/src/compiler/spirv/vtn_private.h @@ -410,8 +410,6 @@ enum vtn_variable_mode { vtn_variable_mode_ubo, vtn_variable_mode_ssbo, vtn_variable_mode_push_constant, - vtn_variable_mode_image, - vtn_variable_mode_sampler, vtn_variable_mode_workgroup, vtn_variable_mode_input, vtn_variable_mode_output, diff --git a/src/compiler/spirv/vtn_variables.c b/src/compiler/spirv/vtn_variables.c index 6b30263645d..225b088ecbc 100644 --- a/src/compiler/spirv/vtn_variables.c +++ b/src/compiler/spirv/vtn_variables.c @@ -1548,9 +1548,7 @@ var_decoration_cb(struct vtn_builder *b, struct vtn_value *val, int member, vtn_var->mode == vtn_variable_mode_output) { is_vertex_input = false; location += vtn_var->patch ? VARYING_SLOT_PATCH0 : VARYING_SLOT_VAR0; - } else if (vtn_var->mode != vtn_variable_mode_uniform && - vtn_var->mode != vtn_variable_mode_sampler && - vtn_var->mode != vtn_variable_mode_image) { + } else if (vtn_var->mode != vtn_variable_mode_uniform) { vtn_warn("Location must be on input, output, uniform, sampler or " "image variable"); return; @@ -1628,12 +1626,7 @@ vtn_storage_class_to_mode(struct vtn_builder *b, nir_mode = 0; break; case SpvStorageClassUniformConstant: - if (glsl_type_is_image(interface_type->type)) - mode = vtn_variable_mode_image; - else if (glsl_type_is_sampler(interface_type->type)) - mode = vtn_variable_mode_sampler; - else - mode = vtn_variable_mode_uniform; + mode = vtn_variable_mode_uniform; nir_mode = nir_var_uniform; break; case SpvStorageClassPushConstant: @@ -1776,11 +1769,11 @@ vtn_create_variable(struct vtn_builder *b, struct vtn_value *val, case vtn_variable_mode_ssbo: b->shader->info.num_ssbos++; break; - case vtn_variable_mode_image: - b->shader->info.num_images++; - break; - case vtn_variable_mode_sampler: - b->shader->info.num_textures++; + case vtn_variable_mode_uniform: + if (glsl_type_is_image(without_array->type)) + b->shader->info.num_images++; + else if (glsl_type_is_sampler(without_array->type)) + b->shader->info.num_textures++; break; case vtn_variable_mode_push_constant: b->shader->num_uniforms = vtn_type_block_size(b, type); @@ -1800,8 +1793,6 @@ vtn_create_variable(struct vtn_builder *b, struct vtn_value *val, switch (var->mode) { case vtn_variable_mode_local: case vtn_variable_mode_global: - case vtn_variable_mode_image: - case vtn_variable_mode_sampler: case vtn_variable_mode_uniform: /* For these, we create the variable normally */ var->var = rzalloc(b->shader, nir_variable); @@ -1809,16 +1800,7 @@ vtn_create_variable(struct vtn_builder *b, struct vtn_value *val, var->var->type = var->type->type; var->var->data.mode = nir_mode; var->var->data.location = -1; - - switch (var->mode) { - case vtn_variable_mode_image: - case vtn_variable_mode_sampler: - var->var->interface_type = without_array->type; - break; - default: - var->var->interface_type = NULL; - break; - } + var->var->interface_type = NULL; break; case vtn_variable_mode_workgroup: @@ -1943,8 +1925,7 @@ vtn_create_variable(struct vtn_builder *b, struct vtn_value *val, vtn_foreach_decoration(b, val, var_decoration_cb, var); - if (var->mode == vtn_variable_mode_image || - var->mode == vtn_variable_mode_sampler) { + if (var->mode == vtn_variable_mode_uniform) { /* XXX: We still need the binding information in the nir_variable * for these. We should fix that. */ @@ -1952,7 +1933,7 @@ vtn_create_variable(struct vtn_builder *b, struct vtn_value *val, var->var->data.descriptor_set = var->descriptor_set; var->var->data.index = var->input_attachment_index; - if (var->mode == vtn_variable_mode_image) + if (glsl_type_is_image(without_array->type)) var->var->data.image.format = without_array->image_format; } @@ -2092,8 +2073,8 @@ vtn_handle_variables(struct vtn_builder *b, SpvOp opcode, vtn_assert_types_equal(b, opcode, res_type, src_val->type->deref); - if (src->mode == vtn_variable_mode_image || - src->mode == vtn_variable_mode_sampler) { + if (glsl_type_is_image(res_type->type) || + glsl_type_is_sampler(res_type->type)) { vtn_push_value(b, w[2], vtn_value_type_pointer)->pointer = src; return; } diff --git a/src/intel/vulkan/anv_nir_apply_pipeline_layout.c b/src/intel/vulkan/anv_nir_apply_pipeline_layout.c index d5a08f712f1..1147e0c9d96 100644 --- a/src/intel/vulkan/anv_nir_apply_pipeline_layout.c +++ b/src/intel/vulkan/anv_nir_apply_pipeline_layout.c @@ -427,10 +427,12 @@ anv_nir_apply_pipeline_layout(struct anv_pipeline *pipeline, } nir_foreach_variable(var, &shader->uniforms) { - if (!glsl_type_is_image(var->interface_type)) + const struct glsl_type *glsl_type = glsl_without_array(var->type); + + if (!glsl_type_is_image(glsl_type)) continue; - enum glsl_sampler_dim dim = glsl_get_sampler_dim(var->interface_type); + enum glsl_sampler_dim dim = glsl_get_sampler_dim(glsl_type); const uint32_t set = var->data.descriptor_set; const uint32_t binding = var->data.binding; -- 2.30.2