spirv: Contain the GLSLang issue #179 workaround to old GLSLang
authorJason Ekstrand <jason.ekstrand@intel.com>
Fri, 11 Jan 2019 21:02:39 +0000 (15:02 -0600)
committerJason Ekstrand <jason.ekstrand@intel.com>
Sat, 12 Jan 2019 23:55:49 +0000 (17:55 -0600)
Instead of applying the workaround universally, detect semi-old GLSLang
via the generator ID and only enable the workaround on old GLSLang.
This isn't nearly as precise as one would like it to be because the
first GLSLang generator id version bump was on October 7, 2017 which is
about 1.5 years after the bug was fixed.  However, it at least lets us
disable it for non-GLSLang and for more modern versions.

Reviewed-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
src/compiler/spirv/spirv_to_nir.c
src/compiler/spirv/vtn_private.h
src/compiler/spirv/vtn_variables.c

index e3dc619c6f7f665777c6def5df2e099eb28f63ae..76a997ee34196f011f23efe4d5dbf2e7ae1202cc 100644 (file)
@@ -4316,6 +4316,15 @@ vtn_create_builder(const uint32_t *words, size_t word_count,
       goto fail;
    }
 
+   uint16_t generator_id = words[2] >> 16;
+   uint16_t generator_version = words[2];
+
+   /* The first GLSLang version bump actually 1.5 years after #179 was fixed
+    * but this should at least let us shut the workaround off for modern
+    * versions of GLSLang.
+    */
+   b->wa_glslang_179 = (generator_id == 8 && generator_version == 1);
+
    /* words[2] == generator magic */
    unsigned value_id_bound = words[3];
    if (words[4] != 0) {
index a54cb8c6495b8b4022353554bf8502570bec3877..09ae8b7145c21e9c46d924c02cfadb9396b76d2c 100644 (file)
@@ -586,6 +586,9 @@ struct vtn_builder {
    unsigned value_id_bound;
    struct vtn_value *values;
 
+   /* True if we should watch out for GLSLang issue #179 */
+   bool wa_glslang_179;
+
    gl_shader_stage entry_point_stage;
    const char *entry_point_name;
    struct vtn_value *entry_point;
index 6036295e61ca90ae5925c4ac5b6914b801a29ce6..6106ff355916ad14d8c625f4373f1fa804070c8e 100644 (file)
@@ -544,9 +544,11 @@ repair_atomic_type(const struct glsl_type *type)
 nir_deref_instr *
 vtn_pointer_to_deref(struct vtn_builder *b, struct vtn_pointer *ptr)
 {
-   /* Do on-the-fly copy propagation for samplers. */
-   if (ptr->var && ptr->var->copy_prop_sampler)
-      return vtn_pointer_to_deref(b, ptr->var->copy_prop_sampler);
+   if (b->wa_glslang_179) {
+      /* Do on-the-fly copy propagation for samplers. */
+      if (ptr->var && ptr->var->copy_prop_sampler)
+         return vtn_pointer_to_deref(b, ptr->var->copy_prop_sampler);
+   }
 
    vtn_assert(!vtn_pointer_uses_ssa_offset(b, ptr));
    if (!ptr->deref) {
@@ -1800,16 +1802,18 @@ vtn_pointer_from_ssa(struct vtn_builder *b, nir_ssa_def *ssa,
    ptr->type = ptr_type->deref;
    ptr->ptr_type = ptr_type;
 
-   /* To work around https://github.com/KhronosGroup/glslang/issues/179 we
-    * need to whack the mode because it creates a function parameter with the
-    * Function storage class even though it's a pointer to a sampler.  If we
-    * don't do this, then NIR won't get rid of the deref_cast for us.
-    */
-   if (ptr->mode == vtn_variable_mode_function &&
-       (ptr->type->base_type == vtn_base_type_sampler ||
-        ptr->type->base_type == vtn_base_type_sampled_image)) {
-      ptr->mode = vtn_variable_mode_uniform;
-      nir_mode = nir_var_uniform;
+   if (b->wa_glslang_179) {
+      /* To work around https://github.com/KhronosGroup/glslang/issues/179 we
+       * need to whack the mode because it creates a function parameter with
+       * the Function storage class even though it's a pointer to a sampler.
+       * If we don't do this, then NIR won't get rid of the deref_cast for us.
+       */
+      if (ptr->mode == vtn_variable_mode_function &&
+          (ptr->type->base_type == vtn_base_type_sampler ||
+           ptr->type->base_type == vtn_base_type_sampled_image)) {
+         ptr->mode = vtn_variable_mode_uniform;
+         nir_mode = nir_var_uniform;
+      }
    }
 
    if (vtn_pointer_uses_ssa_offset(b, ptr)) {
@@ -2266,11 +2270,15 @@ vtn_handle_variables(struct vtn_builder *b, SpvOp opcode,
       vtn_assert_types_equal(b, opcode, dest_val->type->deref, src_val->type);
 
       if (glsl_type_is_sampler(dest->type->type)) {
-         vtn_warn("OpStore of a sampler detected.  Doing on-the-fly copy "
-                  "propagation to workaround the problem.");
-         vtn_assert(dest->var->copy_prop_sampler == NULL);
-         dest->var->copy_prop_sampler =
-            vtn_value(b, w[2], vtn_value_type_pointer)->pointer;
+         if (b->wa_glslang_179) {
+            vtn_warn("OpStore of a sampler detected.  Doing on-the-fly copy "
+                     "propagation to workaround the problem.");
+            vtn_assert(dest->var->copy_prop_sampler == NULL);
+            dest->var->copy_prop_sampler =
+               vtn_value(b, w[2], vtn_value_type_pointer)->pointer;
+         } else {
+            vtn_fail("Vulkan does not allow OpStore of a sampler or image.");
+         }
          break;
       }