[Mesa-dev] [PATCH 09/22] spirv: Get rid of vtn_variable_mode_image/sampler

Alejandro Piñeiro apinheiro at igalia.com
Tue Apr 17 14:42:45 UTC 2018


I forgot to CC any of the radv people. Sorry for the noise.


On 17/04/18 16:36, Alejandro Piñeiro wrote:
> From: Neil Roberts <nroberts at igalia.com>
>
> 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.
>
> Signed-off-by: Eduardo Lima <elima at igalia.com>
> Signed-off-by: Neil Roberts <nroberts at igalia.com
> Signed-off-by: Alejandro Piñeiro <apinheiro at igalia.com>
> ---
>
> Warning to radv people: this commit is a non-trivial change of how
> image/samplers are managed. Specifically, could lead to the interface
> type of image/sampler nir_variable to be NULL. We needed to fix this
> on anv on the following commit "anv/nir: Use nir_variable's type if
> interface_type is null" to avoid some CTS tests to crash. Probably
> radv would need to do something similar.
>
>
>  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 +++++++++++---------------------------
>  4 files changed, 16 insertions(+), 37 deletions(-)
>
> diff --git a/src/compiler/spirv/spirv_to_nir.c b/src/compiler/spirv/spirv_to_nir.c
> index 28274311c2b..b427205affe 100644
> --- a/src/compiler/spirv/spirv_to_nir.c
> +++ b/src/compiler/spirv/spirv_to_nir.c
> @@ -3710,10 +3710,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 e7d2f9ea614..2c3bf698cc2 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 183024e14f4..98bec389fcd 100644
> --- a/src/compiler/spirv/vtn_private.h
> +++ b/src/compiler/spirv/vtn_private.h
> @@ -406,8 +406,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 0996d37e930..633fe6bd9da 100644
> --- a/src/compiler/spirv/vtn_variables.c
> +++ b/src/compiler/spirv/vtn_variables.c
> @@ -1541,9 +1541,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;
> @@ -1621,12 +1619,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:
> @@ -1769,11 +1762,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);
> @@ -1793,8 +1786,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);
> @@ -1802,16 +1793,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:
> @@ -1936,8 +1918,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.
>         */
> @@ -1945,7 +1926,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;
>     }
>  
> @@ -2085,8 +2066,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;
>        }



More information about the mesa-dev mailing list