Remember to add:

--- 8< ---
Fixes the following test:
KHR-GL44.enhanced_layouts.xfb_output_overlapping
--- 8< ---


See below for one addition:

On 4/22/19 3:00 AM, Andres Gomez wrote:
 From page 76 (page 80 of the PDF) of the GLSL 4.60 v.5 spec:

   " No aliasing in output buffers is allowed: It is a compile-time or
     link-time error to specify variables with overlapping transform
     feedback offsets."

Currently, this is expected to fail, but it succeeds:

   "

     ...

     layout (xfb_offset = 0) out vec2 a;
     layout (xfb_offset = 0) out vec4 b;

     ...

   "

v2:
   - Use a data structure to track the used components instead of a
     nested loop (Ilia).

v3:
   - Take the BITSET_WORD array out from the
     gl_transform_feedback_buffer struct and make it local to the
     validation process (Timothy).
   - Do not use a nested scope for the validation (Timothy).

Cc: Timothy Arceri <tarc...@itsqueeze.com>
Cc: Ilia Mirkin <imir...@alum.mit.edu>
Signed-off-by: Andres Gomez <ago...@igalia.com>
---
  src/compiler/glsl/link_varyings.cpp | 109 ++++++++++++++++++++--------
  src/compiler/glsl/link_varyings.h   |   6 +-
  2 files changed, 84 insertions(+), 31 deletions(-)

diff --git a/src/compiler/glsl/link_varyings.cpp 
b/src/compiler/glsl/link_varyings.cpp
index 22ec411837d..89874530980 100644
--- a/src/compiler/glsl/link_varyings.cpp
+++ b/src/compiler/glsl/link_varyings.cpp
@@ -1169,8 +1169,10 @@ bool
  tfeedback_decl::store(struct gl_context *ctx, struct gl_shader_program *prog,
                        struct gl_transform_feedback_info *info,
                        unsigned buffer, unsigned buffer_index,
-                      const unsigned max_outputs, bool *explicit_stride,
-                      bool has_xfb_qualifiers) const
+                      const unsigned max_outputs,
+                      BITSET_WORD *used_components[MAX_FEEDBACK_BUFFERS],
+                      bool *explicit_stride, bool has_xfb_qualifiers,
+                      const void* mem_ctx) const
  {
     unsigned xfb_offset = 0;
     unsigned size = this->size;
@@ -1197,6 +1199,72 @@ tfeedback_decl::store(struct gl_context *ctx, struct 
gl_shader_program *prog,
        unsigned location = this->location;
        unsigned location_frac = this->location_frac;
        unsigned num_components = this->num_components();
+
+      /* From GL_EXT_transform_feedback:
+       *
+       *   " A program will fail to link if:
+       *
+       *       * the total number of components to capture is greater than the
+       *         constant MAX_TRANSFORM_FEEDBACK_INTERLEAVED_COMPONENTS_EXT
+       *         and the buffer mode is INTERLEAVED_ATTRIBS_EXT."
+       *
+       * From GL_ARB_enhanced_layouts:
+       *
+       *   " The resulting stride (implicit or explicit) must be less than or
+       *     equal to the implementation-dependent constant
+       *     gl_MaxTransformFeedbackInterleavedComponents."
+       */
+      if ((prog->TransformFeedback.BufferMode == GL_INTERLEAVED_ATTRIBS ||
+           has_xfb_qualifiers) &&
+          xfb_offset + num_components >
+          ctx->Const.MaxTransformFeedbackInterleavedComponents) {
+         linker_error(prog,
+                      "The MAX_TRANSFORM_FEEDBACK_INTERLEAVED_COMPONENTS "
+                      "limit has been exceeded.");
+         return false;
+      }
+
+      /* From the OpenGL 4.60.5 spec, section 4.4.2. Output Layout Qualifiers,
+       * Page 76, (Transform Feedback Layout Qualifiers):
+       *
+       *   " No aliasing in output buffers is allowed: It is a compile-time or
+       *     link-time error to specify variables with overlapping transform
+       *     feedback offsets."
+       */
+      const unsigned max_components =
+         ctx->Const.MaxTransformFeedbackInterleavedComponents;
+      const unsigned first_component = xfb_offset;
+      const unsigned last_component = xfb_offset + num_components - 1;
+      const unsigned start_word = BITSET_BITWORD(first_component);
+      const unsigned end_word = BITSET_BITWORD(last_component);
+      BITSET_WORD *used;
+      assert(last_component < max_components);
+
+      if (!used_components[buffer]) {
+         used_components[buffer] =
+            rzalloc_array(mem_ctx, BITSET_WORD, BITSET_WORDS(max_components));
+      }
+      used = used_components[buffer];
+
+      for (unsigned word = start_word; word <= end_word; word++) {
+         unsigned start_range = 0;
+         unsigned end_range = BITSET_WORDBITS - 1;
+
+         if (word == start_word)
+            start_range = first_component % BITSET_WORDBITS;
+
+         if (word == end_word)
+            end_range = last_component % BITSET_WORDBITS;
+
+         if (used[word] & BITSET_RANGE(start_range, end_range)) {
+            linker_error(prog,
+                         "variable '%s', xfb_offset (%d) is causing aliasing.",
+                         this->orig_name, xfb_offset * 4);
+            return false;
+         }
+         used[word] |= BITSET_RANGE(start_range, end_range);
+      }
+
        while (num_components > 0) {
           unsigned output_size = MIN2(num_components, 4 - location_frac);
           assert((info->NumOutputs == 0 && max_outputs == 0) ||
@@ -1247,28 +1315,6 @@ tfeedback_decl::store(struct gl_context *ctx, struct 
gl_shader_program *prog,
        info->Buffers[buffer].Stride = xfb_offset;
     }
- /* From GL_EXT_transform_feedback:
-    *   A program will fail to link if:
-    *
-    *     * the total number of components to capture is greater than
-    *       the constant MAX_TRANSFORM_FEEDBACK_INTERLEAVED_COMPONENTS_EXT
-    *       and the buffer mode is INTERLEAVED_ATTRIBS_EXT.
-    *
-    * From GL_ARB_enhanced_layouts:
-    *
-    *   "The resulting stride (implicit or explicit) must be less than or
-    *   equal to the implementation-dependent constant
-    *   gl_MaxTransformFeedbackInterleavedComponents."
-    */
-   if ((prog->TransformFeedback.BufferMode == GL_INTERLEAVED_ATTRIBS ||
-        has_xfb_qualifiers) &&
-       info->Buffers[buffer].Stride >
-       ctx->Const.MaxTransformFeedbackInterleavedComponents) {
-      linker_error(prog, "The MAX_TRANSFORM_FEEDBACK_INTERLEAVED_COMPONENTS "
-                   "limit has been exceeded.");
-      return false;
-   }
-
   store_varying:
     info->Varyings[info->NumVarying].Name = ralloc_strdup(prog,
                                                           this->orig_name);
@@ -1389,7 +1435,8 @@ cmp_xfb_offset(const void * x_generic, const void * 
y_generic)
  static bool
  store_tfeedback_info(struct gl_context *ctx, struct gl_shader_program *prog,
                       unsigned num_tfeedback_decls,
-                     tfeedback_decl *tfeedback_decls, bool has_xfb_qualifiers)
+                     tfeedback_decl *tfeedback_decls, bool has_xfb_qualifiers,
+                     const void *mem_ctx)
  {
     if (!prog->last_vert_prog)
        return true;
@@ -1431,6 +1478,7 @@ store_tfeedback_info(struct gl_context *ctx, struct 
gl_shader_program *prog,
unsigned num_buffers = 0;
     unsigned buffers = 0;
+   BITSET_WORD *used_components[MAX_FEEDBACK_BUFFERS];

You need to initialize it here, otherwise things end up bad:

BITSET_WORD *used_components[MAX_FEEDBACK_BUFFERS] = {};

With this change;
Reviewed-by: Tapani Pälli <tapani.pa...@intel.com>


     if (!has_xfb_qualifiers && separate_attribs_mode) {
        /* GL_SEPARATE_ATTRIBS */
@@ -1438,7 +1486,8 @@ store_tfeedback_info(struct gl_context *ctx, struct 
gl_shader_program *prog,
           if (!tfeedback_decls[i].store(ctx, prog,
                                         xfb_prog->sh.LinkedTransformFeedback,
                                         num_buffers, num_buffers, num_outputs,
-                                       NULL, has_xfb_qualifiers))
+                                       used_components, NULL,
+                                       has_xfb_qualifiers, mem_ctx))
              return false;
buffers |= 1 << num_buffers;
@@ -1475,7 +1524,8 @@ store_tfeedback_info(struct gl_context *ctx, struct 
gl_shader_program *prog,
              if (!tfeedback_decls[i].store(ctx, prog,
                                            
xfb_prog->sh.LinkedTransformFeedback,
                                            buffer, num_buffers, num_outputs,
-                                          explicit_stride, has_xfb_qualifiers))
+                                          used_components, explicit_stride,
+                                          has_xfb_qualifiers, mem_ctx))
                 return false;
              num_buffers++;
              buffer_stream_id = -1;
@@ -1516,7 +1566,8 @@ store_tfeedback_info(struct gl_context *ctx, struct 
gl_shader_program *prog,
           if (!tfeedback_decls[i].store(ctx, prog,
                                         xfb_prog->sh.LinkedTransformFeedback,
                                         buffer, num_buffers, num_outputs,
-                                       explicit_stride, has_xfb_qualifiers))
+                                       used_components, explicit_stride,
+                                       has_xfb_qualifiers, mem_ctx))
              return false;
        }
     }
@@ -3002,7 +3053,7 @@ link_varyings(struct gl_shader_program *prog, unsigned 
first, unsigned last,
     }
if (!store_tfeedback_info(ctx, prog, num_tfeedback_decls, tfeedback_decls,
-                             has_xfb_qualifiers))
+                             has_xfb_qualifiers, mem_ctx))
        return false;
return true;
diff --git a/src/compiler/glsl/link_varyings.h 
b/src/compiler/glsl/link_varyings.h
index d0f7ca355b8..b802250819e 100644
--- a/src/compiler/glsl/link_varyings.h
+++ b/src/compiler/glsl/link_varyings.h
@@ -34,7 +34,7 @@
#include "main/glheader.h"
  #include "program/prog_parameter.h"
-
+#include "util/bitset.h"
struct gl_shader_program;
  struct gl_shader;
@@ -99,7 +99,9 @@ public:
     bool store(struct gl_context *ctx, struct gl_shader_program *prog,
                struct gl_transform_feedback_info *info, unsigned buffer,
                unsigned buffer_index, const unsigned max_outputs,
-              bool *explicit_stride, bool has_xfb_qualifiers) const;
+              BITSET_WORD *used_components[MAX_FEEDBACK_BUFFERS],
+              bool *explicit_stride, bool has_xfb_qualifiers,
+              const void *mem_ctx) const;
     const tfeedback_candidate *find_candidate(gl_shader_program *prog,
                                               hash_table 
*tfeedback_candidates);
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Reply via email to