[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:36:46 UTC 2018


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;
       }
-- 
2.14.1



More information about the mesa-dev mailing list