Mesa (master): glsl/linker: Fix xfb with explicit locations and 64bit types

GitLab Mirror gitlab-mirror at kemper.freedesktop.org
Thu Apr 1 17:39:01 UTC 2021


Module: Mesa
Branch: master
Commit: 67e33db4a6c302f09ba74a534aa4b18815239aa1
URL:    http://cgit.freedesktop.org/mesa/mesa/commit/?id=67e33db4a6c302f09ba74a534aa4b18815239aa1

Author: Danylo Piliaiev <danylo.piliaiev at globallogic.com>
Date:   Thu Oct 10 19:11:00 2019 +0300

glsl/linker: Fix xfb with explicit locations and 64bit types

1) Per GL_ARB_enhanced_layouts if explicit location is set for varying,
each struct member, array element and matrix row will take
separate location. With GL_ARB_gpu_shader_fp64/GL_ARB_gpu_shader_int64
they may take two locations.

Examples:

| layout(location=0) dvec3[2] a; | layout(location=4) vec2[4] b; |
|                                |                               |
|   32b 32b 32b 32b              |   32b 32b 32b 32b             |
| 0  X   X   Y   Y               | 4  X   Y   0   0              |
| 1  Z   Z   0   0               | 5  X   Y   0   0              |
| 2  X   X   Y   Y               | 6  X   Y   0   0              |
| 3  Z   Z   0   0               | 7  X   Y   0   0              |

Previously it wasn't taken into account.

2) Captured double-precision variables should be aligned to
8 bytes per GL_ARB_gpu_shader_fp64:
 "If any variable captured in transform feedback has double-precision
 components, the practical requirements for defined behavior are:
     ...
 (c) each double-precision variable captured must be aligned to a
     multiple of eight bytes relative to the beginning of a vertex."

v2: fix `output_size` calculations
         ( Andrii Simiklit <andrii.simiklit at globallogic.com> )

Closes: https://gitlab.freedesktop.org/mesa/mesa/issues/1667
Reviewed-by: Tapani Pälli <tapani.palli at intel.com>
Signed-off-by: Danylo Piliaiev <danylo.piliaiev at globallogic.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/2333>

---

 src/compiler/glsl/link_varyings.cpp | 114 ++++++++++++++++++++++++++++++++----
 src/compiler/glsl/link_varyings.h   |  12 +++-
 2 files changed, 111 insertions(+), 15 deletions(-)

diff --git a/src/compiler/glsl/link_varyings.cpp b/src/compiler/glsl/link_varyings.cpp
index 0080c2451fb..2f59967339e 100644
--- a/src/compiler/glsl/link_varyings.cpp
+++ b/src/compiler/glsl/link_varyings.cpp
@@ -65,6 +65,13 @@ get_varying_type(const ir_variable *var, gl_shader_stage stage)
    return type;
 }
 
+static bool
+varying_has_user_specified_location(const ir_variable *var)
+{
+   return var->data.explicit_location &&
+      var->data.location >= VARYING_SLOT_VAR0;
+}
+
 static void
 create_xfb_varying_names(void *mem_ctx, const glsl_type *t, char **name,
                          size_t name_length, unsigned *count,
@@ -1081,7 +1088,7 @@ tfeedback_decl::assign_location(struct gl_context *ctx,
    unsigned fine_location
       = this->matched_candidate->toplevel_var->data.location * 4
       + this->matched_candidate->toplevel_var->data.location_frac
-      + this->matched_candidate->offset;
+      + this->matched_candidate->struct_offset_floats;
    const unsigned dmul =
       this->matched_candidate->type->without_array()->is_64bit() ? 2 : 1;
 
@@ -1174,7 +1181,7 @@ tfeedback_decl::assign_location(struct gl_context *ctx,
    this->stream_id = this->matched_candidate->toplevel_var->data.stream;
 
    unsigned array_offset = this->array_subscript * 4 * dmul;
-   unsigned struct_offset = this->matched_candidate->offset * 4 * dmul;
+   unsigned struct_offset = this->matched_candidate->xfb_offset_floats * 4;
    this->buffer = this->matched_candidate->toplevel_var->data.xfb_buffer;
    this->offset = this->matched_candidate->toplevel_var->data.offset +
       array_offset + struct_offset;
@@ -1189,7 +1196,14 @@ tfeedback_decl::get_num_outputs() const
    if (!this->is_varying()) {
       return 0;
    }
-   return (this->num_components() + this->location_frac + 3)/4;
+
+   if (varying_has_user_specified_location(this->matched_candidate->toplevel_var)) {
+      unsigned dmul = this->is_64bit() ? 2 : 1;
+      unsigned rows_per_element = DIV_ROUND_UP(this->vector_elements * dmul, 4);
+      return this->size * this->matrix_columns * rows_per_element;
+   } else {
+      return (this->num_components() + this->location_frac + 3) / 4;
+   }
 }
 
 
@@ -1299,8 +1313,50 @@ tfeedback_decl::store(struct gl_context *ctx, struct gl_shader_program *prog,
          used[word] |= BITSET_RANGE(start_range, end_range);
       }
 
+      const unsigned type_num_components =
+         this->vector_elements * (this->is_64bit() ? 2 : 1);
+      unsigned current_type_components_left = type_num_components;
+
       while (num_components > 0) {
-         unsigned output_size = MIN2(num_components, 4 - location_frac);
+         unsigned output_size = 0;
+
+         /*  From GL_ARB_enhanced_layouts:
+          *
+          * "When an attribute variable declared using an array type is bound to
+          * generic attribute index <i>, the active array elements are assigned to
+          * consecutive generic attributes beginning with generic attribute <i>.  The
+          * number of attributes and components assigned to each element are
+          * determined according to the data type of array elements and "component"
+          * layout qualifier (if any) specified in the declaration of the array."
+          *
+          * "When an attribute variable declared using a matrix type is bound to a
+          * generic attribute index <i>, its values are taken from consecutive generic
+          * attributes beginning with generic attribute <i>.  Such matrices are
+          * treated as an array of column vectors with values taken from the generic
+          * attributes.
+          * This means there may be gaps in the varyings we are taking values from."
+          *
+          * Examples:
+          *
+          * | layout(location=0) dvec3[2] a; | layout(location=4) vec2[4] b; |
+          * |                                |                               |
+          * |        32b 32b 32b 32b         |        32b 32b 32b 32b        |
+          * |      0  X   X   Y   Y          |      4  X   Y   0   0         |
+          * |      1  Z   Z   0   0          |      5  X   Y   0   0         |
+          * |      2  X   X   Y   Y          |      6  X   Y   0   0         |
+          * |      3  Z   Z   0   0          |      7  X   Y   0   0         |
+          *
+          */
+         if (varying_has_user_specified_location(this->matched_candidate->toplevel_var)) {
+            output_size = MIN3(num_components, current_type_components_left, 4);
+            current_type_components_left -= output_size;
+            if (current_type_components_left == 0) {
+               current_type_components_left = type_num_components;
+            }
+         } else {
+            output_size = MIN2(num_components, 4 - location_frac);
+         }
+
          assert((info->NumOutputs == 0 && max_outputs == 0) ||
                 info->NumOutputs < max_outputs);
 
@@ -2352,7 +2408,8 @@ public:
         tfeedback_candidates(tfeedback_candidates),
         stage(stage),
         toplevel_var(NULL),
-        varying_floats(0)
+        varying_floats(0),
+        xfb_offset_floats(0)
    {
    }
 
@@ -2364,6 +2421,7 @@ public:
 
       this->toplevel_var = var;
       this->varying_floats = 0;
+      this->xfb_offset_floats = 0;
       const glsl_type *t =
          var->data.from_named_ifc_block ? var->get_interface_type() : var->type;
       if (!var->data.patch && stage == MESA_SHADER_TESS_CTRL) {
@@ -2387,11 +2445,37 @@ private:
          = rzalloc(this->mem_ctx, tfeedback_candidate);
       candidate->toplevel_var = this->toplevel_var;
       candidate->type = type;
-      candidate->offset = this->varying_floats;
-      _mesa_hash_table_insert(this->tfeedback_candidates,
-                              ralloc_strdup(this->mem_ctx, name),
-                              candidate);
-      this->varying_floats += type->component_slots();
+
+      if (type->without_array()->is_64bit()) {
+         /*  From ARB_gpu_shader_fp64:
+          *
+          * If any variable captured in transform feedback has double-precision
+          * components, the practical requirements for defined behavior are:
+          *     ...
+          * (c) each double-precision variable captured must be aligned to a
+          *     multiple of eight bytes relative to the beginning of a vertex.
+          */
+         this->xfb_offset_floats = ALIGN(this->xfb_offset_floats, 2);
+         /* 64-bit members of structs are also aligned. */
+         this->varying_floats = ALIGN(this->varying_floats, 2);
+      }
+
+      candidate->xfb_offset_floats = this->xfb_offset_floats;
+      candidate->struct_offset_floats = this->varying_floats;
+
+       _mesa_hash_table_insert(this->tfeedback_candidates,
+                               ralloc_strdup(this->mem_ctx, name),
+                               candidate);
+
+      const unsigned component_slots = type->component_slots();
+
+      if (varying_has_user_specified_location(this->toplevel_var)) {
+         this->varying_floats += type->count_attribute_slots(false) * 4;
+      } else {
+         this->varying_floats += component_slots;
+      }
+
+      this->xfb_offset_floats += component_slots;
    }
 
    /**
@@ -2417,6 +2501,11 @@ private:
     * variable.
     */
    unsigned varying_floats;
+
+   /**
+    * Offset within the xfb. Counted in floats.
+    */
+   unsigned xfb_offset_floats;
 };
 
 
@@ -2835,7 +2924,7 @@ assign_varying_locations(struct gl_context *ctx,
          matched_candidate->type->without_array()->is_64bit() ? 2 : 1;
       const bool lowered =
          (disable_xfb_packing &&
-          !tfeedback_decls[i].is_aligned(dmul, matched_candidate->offset)) ||
+          !tfeedback_decls[i].is_aligned(dmul, matched_candidate->struct_offset_floats)) ||
          (matched_candidate->toplevel_var->data.explicit_location &&
           matched_candidate->toplevel_var->data.location < VARYING_SLOT_VAR0 &&
           (!consumer || consumer->Stage == MESA_SHADER_FRAGMENT) &&
@@ -2857,7 +2946,8 @@ assign_varying_locations(struct gl_context *ctx,
          new_candidate->toplevel_var = new_var;
          new_candidate->toplevel_var->data.is_unmatched_generic_inout = 1;
          new_candidate->type = new_var->type;
-         new_candidate->offset = 0;
+         new_candidate->struct_offset_floats = 0;
+         new_candidate->xfb_offset_floats = 0;
          _mesa_hash_table_insert(tfeedback_candidates,
                                  ralloc_strdup(mem_ctx, new_var->name),
                                  new_candidate);
diff --git a/src/compiler/glsl/link_varyings.h b/src/compiler/glsl/link_varyings.h
index 6f4bcdc79c5..7bad222f132 100644
--- a/src/compiler/glsl/link_varyings.h
+++ b/src/compiler/glsl/link_varyings.h
@@ -77,10 +77,16 @@ struct tfeedback_candidate
    const glsl_type *type;
 
    /**
-    * Offset within the toplevel variable where this varying occurs (counted
-    * in multiples of the size of a float).
+    * Offset within the toplevel variable where this varying occurs.
+    * Counted in floats.
     */
-   unsigned offset;
+   unsigned struct_offset_floats;
+
+   /**
+    * Offset within the xfb with respect to alignment requirements.
+    * Counted in floats.
+    */
+   unsigned xfb_offset_floats;
 };
 
 



More information about the mesa-commit mailing list