nir/spirv: Add better location handling

Previously, our location handling was focussed on either no location
(usually implicit 0) or a builting.  Unfortunately, if you gave it a
location, it would blow it away and just not care.  This worked fine with
crucible and our meta shaders but didn't work with the CTS.  The new code
uses the "data.explicit_location" field to denote that it has a "final"
location (usually from a builtin) and, otherwise, the location is
considered to be relative to the base for that shader stage.

diff --git a/src/glsl/nir/spirv_to_nir.c b/src/glsl/nir/spirv_to_nir.c
index e6fe74de9e86ecb9d5a7fdb67a98bda4c3995879..60e7a338ee1c5ec2c8f32e38599401cf49f5670a 100644 (file)
--- a/src/glsl/nir/spirv_to_nir.c
+++ b/src/glsl/nir/spirv_to_nir.c
@@ -760,7 +760,6 @@ var_decoration_cb(struct vtn_builder *b, struct vtn_value *val, int member,
       var->data.read_only = true;
       break;
    case SpvDecorationLocation:
-      var->data.explicit_location = true;
       var->data.location = dec->literals[0];
       break;
    case SpvDecorationComponent:
@@ -781,6 +780,7 @@ var_decoration_cb(struct vtn_builder *b, struct vtn_value *val, int member,
       nir_variable_mode mode;
       vtn_get_builtin_location(dec->literals[0], &var->data.location,
                                &mode);
+      var->data.explicit_location = true;
       var->data.mode = mode;
       if (mode == nir_var_shader_in || mode == nir_var_system_value)
          var->data.read_only = true;
@@ -830,6 +830,7 @@ get_builtin_variable(struct vtn_builder *b,
 
       nir_variable_mode mode;
       vtn_get_builtin_location(builtin, &var->data.location, &mode);
+      var->data.explicit_location = true;
       var->data.mode = mode;
       var->name = ralloc_strdup(var, "builtin");
 
@@ -1282,23 +1283,27 @@ vtn_handle_variables(struct vtn_builder *b, SpvOp opcode,
       val->deref = nir_deref_var_create(b, var);
       val->deref_type = type;
 
-      if (b->execution_model == SpvExecutionModelFragment &&
-          var->data.mode == nir_var_shader_out) {
-         var->data.location += FRAG_RESULT_DATA0;
-      } else if (b->execution_model == SpvExecutionModelVertex &&
-                 var->data.mode == nir_var_shader_in) {
-         var->data.location += VERT_ATTRIB_GENERIC0;
-      } else if (var->data.mode == nir_var_shader_in ||
-                 var->data.mode == nir_var_shader_out) {
-         var->data.location += VARYING_SLOT_VAR0;
-      }
-
-      /* We handle decorations last because decorations might cause us to
-       * over-write other things such as the variable's location and we want
-       * those changes to stick.
+      /* We handle decorations first because decorations might give us
+       * location information.  We use the data.explicit_location field to
+       * note that the location provided is the "final" location.  If
+       * data.explicit_location == false, this means that it's relative to
+       * whatever the base location is.
        */
       vtn_foreach_decoration(b, val, var_decoration_cb, var);
 
+      if (!var->data.explicit_location) {
+         if (b->execution_model == SpvExecutionModelFragment &&
+             var->data.mode == nir_var_shader_out) {
+            var->data.location += FRAG_RESULT_DATA0;
+         } else if (b->execution_model == SpvExecutionModelVertex &&
+                    var->data.mode == nir_var_shader_in) {
+            var->data.location += VERT_ATTRIB_GENERIC0;
+         } else if (var->data.mode == nir_var_shader_in ||
+                    var->data.mode == nir_var_shader_out) {
+            var->data.location += VARYING_SLOT_VAR0;
+         }
+      }
+
       /* If this was a uniform block, then we're not going to actually use the
        * variable (we're only going to use it to compute offsets), so don't
        * declare it in the shader.