[v2,6/6] glsl/linker: check for xfb_offset aliasing

Submitted by Andres Gomez on Feb. 12, 2019, 9:57 p.m.

Details

Message ID 20190212215717.11693-1-agomez@igalia.com
State Superseded
Headers show
Series "glsl/linker: several arb_enhanced_layouts related fixes" ( rev: 4 3 ) in Mesa

Not browsing as part of any series.

Commit Message

Andres Gomez Feb. 12, 2019, 9:57 p.m.
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).

Cc: Timothy Arceri <tarceri@itsqueeze.com>
Cc: Ilia Mirkin <imirkin@alum.mit.edu>
Signed-off-by: Andres Gomez <agomez@igalia.com>
---
 src/compiler/glsl/link_varyings.cpp | 89 ++++++++++++++++++++++-------
 src/mesa/main/mtypes.h              |  3 +
 2 files changed, 70 insertions(+), 22 deletions(-)

Patch hide | download patch | download mbox

diff --git a/src/compiler/glsl/link_varyings.cpp b/src/compiler/glsl/link_varyings.cpp
index 8c7d6c14c8f..95e9ae895d2 100644
--- a/src/compiler/glsl/link_varyings.cpp
+++ b/src/compiler/glsl/link_varyings.cpp
@@ -1173,6 +1173,73 @@  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);
+         assert(last_component < max_components);
+
+         if (!info->Buffers[buffer].UsedComponents) {
+            info->Buffers[buffer].UsedComponents =
+               rzalloc_array(info, BITSET_WORD, BITSET_WORDS(max_components));
+         }
+         BITSET_WORD *used = info->Buffers[buffer].UsedComponents;
+
+         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) ||
@@ -1223,28 +1290,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);
diff --git a/src/mesa/main/mtypes.h b/src/mesa/main/mtypes.h
index dda96cd2f19..a32128ec6b9 100644
--- a/src/mesa/main/mtypes.h
+++ b/src/mesa/main/mtypes.h
@@ -49,6 +49,7 @@ 
 #include "compiler/glsl/list.h"
 #include "util/simple_mtx.h"
 #include "util/u_dynarray.h"
+#include "util/bitset.h"
 
 
 #ifdef __cplusplus
@@ -1764,6 +1765,8 @@  struct gl_transform_feedback_buffer
 
    uint32_t NumVaryings;
 
+   BITSET_WORD *UsedComponents;
+
    /**
     * Total number of components stored in each buffer.  This may be used by
     * hardware back-ends to determine the correct stride when interleaving

Comments

I'll add locally to the commit log:

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

On Tue, 2019-02-12 at 23:57 +0200, 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).
> 
> Cc: Timothy Arceri <tarceri@itsqueeze.com>
> Cc: Ilia Mirkin <imirkin@alum.mit.edu>
> Signed-off-by: Andres Gomez <agomez@igalia.com>
> ---
>  src/compiler/glsl/link_varyings.cpp | 89 ++++++++++++++++++++++-------
>  src/mesa/main/mtypes.h              |  3 +
>  2 files changed, 70 insertions(+), 22 deletions(-)
> 
> diff --git a/src/compiler/glsl/link_varyings.cpp b/src/compiler/glsl/link_varyings.cpp
> index 8c7d6c14c8f..95e9ae895d2 100644
> --- a/src/compiler/glsl/link_varyings.cpp
> +++ b/src/compiler/glsl/link_varyings.cpp
> @@ -1173,6 +1173,73 @@ 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);
> +         assert(last_component < max_components);
> +
> +         if (!info->Buffers[buffer].UsedComponents) {
> +            info->Buffers[buffer].UsedComponents =
> +               rzalloc_array(info, BITSET_WORD, BITSET_WORDS(max_components));
> +         }
> +         BITSET_WORD *used = info->Buffers[buffer].UsedComponents;
> +
> +         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) ||
> @@ -1223,28 +1290,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);
> diff --git a/src/mesa/main/mtypes.h b/src/mesa/main/mtypes.h
> index dda96cd2f19..a32128ec6b9 100644
> --- a/src/mesa/main/mtypes.h
> +++ b/src/mesa/main/mtypes.h
> @@ -49,6 +49,7 @@
>  #include "compiler/glsl/list.h"
>  #include "util/simple_mtx.h"
>  #include "util/u_dynarray.h"
> +#include "util/bitset.h"
>  
>  
>  #ifdef __cplusplus
> @@ -1764,6 +1765,8 @@ struct gl_transform_feedback_buffer
>  
>     uint32_t NumVaryings;
>  
> +   BITSET_WORD *UsedComponents;
> +
>     /**
>      * Total number of components stored in each buffer.  This may be used by
>      * hardware back-ends to determine the correct stride when interleaving
On 13/2/19 8:57 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).
> 
> Cc: Timothy Arceri <tarceri@itsqueeze.com>
> Cc: Ilia Mirkin <imirkin@alum.mit.edu>
> Signed-off-by: Andres Gomez <agomez@igalia.com>
> ---
>   src/compiler/glsl/link_varyings.cpp | 89 ++++++++++++++++++++++-------
>   src/mesa/main/mtypes.h              |  3 +
>   2 files changed, 70 insertions(+), 22 deletions(-)
> 
> diff --git a/src/compiler/glsl/link_varyings.cpp b/src/compiler/glsl/link_varyings.cpp
> index 8c7d6c14c8f..95e9ae895d2 100644
> --- a/src/compiler/glsl/link_varyings.cpp
> +++ b/src/compiler/glsl/link_varyings.cpp
> @@ -1173,6 +1173,73 @@ 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;
> +      }
> +
> +      {

We don't need to do this in Mesa anymore. All compilers are now expected 
to support C99 i.e. variable declaration is no longer restricted to the 
start of a compound statement.

> +         /* 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);
> +         assert(last_component < max_components);
> +
> +         if (!info->Buffers[buffer].UsedComponents) {
> +            info->Buffers[buffer].UsedComponents =
> +               rzalloc_array(info, BITSET_WORD, BITSET_WORDS(max_components));
> +         }
> +         BITSET_WORD *used = info->Buffers[buffer].UsedComponents;
> +
> +         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) ||
> @@ -1223,28 +1290,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);
> diff --git a/src/mesa/main/mtypes.h b/src/mesa/main/mtypes.h
> index dda96cd2f19..a32128ec6b9 100644
> --- a/src/mesa/main/mtypes.h
> +++ b/src/mesa/main/mtypes.h
> @@ -49,6 +49,7 @@
>   #include "compiler/glsl/list.h"
>   #include "util/simple_mtx.h"
>   #include "util/u_dynarray.h"
> +#include "util/bitset.h"
>   
>   
>   #ifdef __cplusplus
> @@ -1764,6 +1765,8 @@ struct gl_transform_feedback_buffer
>   
>      uint32_t NumVaryings;
>   
> +   BITSET_WORD *UsedComponents;

Please don't add new members here that are used only for compile time 
validation. Can be try to come up with a different solution to this please?

> +
>      /**
>       * Total number of components stored in each buffer.  This may be used by
>       * hardware back-ends to determine the correct stride when interleaving
>
On Wed, 2019-04-10 at 10:40 +1000, Timothy Arceri wrote:
> On 13/2/19 8:57 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).
> > 
> > Cc: Timothy Arceri <tarceri@itsqueeze.com>
> > Cc: Ilia Mirkin <imirkin@alum.mit.edu>
> > Signed-off-by: Andres Gomez <agomez@igalia.com>
> > ---
> >   src/compiler/glsl/link_varyings.cpp | 89 ++++++++++++++++++++++-------
> >   src/mesa/main/mtypes.h              |  3 +
> >   2 files changed, 70 insertions(+), 22 deletions(-)
> > 
> > diff --git a/src/compiler/glsl/link_varyings.cpp b/src/compiler/glsl/link_varyings.cpp
> > index 8c7d6c14c8f..95e9ae895d2 100644
> > --- a/src/compiler/glsl/link_varyings.cpp
> > +++ b/src/compiler/glsl/link_varyings.cpp
> > @@ -1173,6 +1173,73 @@ 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;
> > +      }
> > +
> > +      {
> 
> We don't need to do this in Mesa anymore. All compilers are now expected 
> to support C99 i.e. variable declaration is no longer restricted to the 
> start of a compound statement.

I can remove the brackets but I don't need those variables to outlive
beyond this piece of code for the validation either.

> 
> > +         /* 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);
> > +         assert(last_component < max_components);
> > +
> > +         if (!info->Buffers[buffer].UsedComponents) {
> > +            info->Buffers[buffer].UsedComponents =
> > +               rzalloc_array(info, BITSET_WORD, BITSET_WORDS(max_components));
> > +         }
> > +         BITSET_WORD *used = info->Buffers[buffer].UsedComponents;
> > +
> > +         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) ||
> > @@ -1223,28 +1290,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);
> > diff --git a/src/mesa/main/mtypes.h b/src/mesa/main/mtypes.h
> > index dda96cd2f19..a32128ec6b9 100644
> > --- a/src/mesa/main/mtypes.h
> > +++ b/src/mesa/main/mtypes.h
> > @@ -49,6 +49,7 @@
> >   #include "compiler/glsl/list.h"
> >   #include "util/simple_mtx.h"
> >   #include "util/u_dynarray.h"
> > +#include "util/bitset.h"
> >   
> >   
> >   #ifdef __cplusplus
> > @@ -1764,6 +1765,8 @@ struct gl_transform_feedback_buffer
> >   
> >      uint32_t NumVaryings;
> >   
> > +   BITSET_WORD *UsedComponents;
> 
> Please don't add new members here that are used only for compile time 
> validation. Can be try to come up with a different solution to this please?

v1 was trying to do this through a nested loop to avoid adding extra
data structures. IMO, the nested loop had a near O(N^2) behavior but
given the possible real max amount of loops, it shouldn't have been too
bad.

However, Ilia didn't like it and suggested an array-based solution. I'm
using here a bitmask and the max amount of components would make this
to not use a lot of memory.

I think both solutions are fine but, if I have to come up with
something else, I'm running out of ideas ...

Suggestions are welcome! ☺

> 
> > +
> >      /**
> >       * Total number of components stored in each buffer.  This may be used by
> >       * hardware back-ends to determine the correct stride when interleaving
> >
On Wed, Apr 10, 2019 at 7:31 AM Andres Gomez <agomez@igalia.com> wrote:
> On Wed, 2019-04-10 at 10:40 +1000, Timothy Arceri wrote:
> > On 13/2/19 8:57 am, Andres Gomez wrote:
> > > @@ -1764,6 +1765,8 @@ struct gl_transform_feedback_buffer
> > >
> > >      uint32_t NumVaryings;
> > >
> > > +   BITSET_WORD *UsedComponents;
> >
> > Please don't add new members here that are used only for compile time
> > validation. Can be try to come up with a different solution to this please?
>
> v1 was trying to do this through a nested loop to avoid adding extra
> data structures. IMO, the nested loop had a near O(N^2) behavior but
> given the possible real max amount of loops, it shouldn't have been too
> bad.
>
> However, Ilia didn't like it and suggested an array-based solution. I'm
> using here a bitmask and the max amount of components would make this
> to not use a lot of memory.
>
> I think both solutions are fine but, if I have to come up with
> something else, I'm running out of ideas ...
>
> Suggestions are welcome!

I believe the comment is that this shouldn't live in "struct
gl_transform_feedback_buffer" but rather in tfeedback_decl somewhere.
That should be easy to do, no? (I haven't gone back and looked at the
code, so perhaps not?)

  -ilia