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

Submitted by Andres Gomez on Feb. 1, 2019, 6:05 p.m.

Details

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

Not browsing as part of any series.

Commit Message

Andres Gomez Feb. 1, 2019, 6:05 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;

    ...

  "

Cc: Timothy Arceri <tarceri@itsqueeze.com>
Signed-off-by: Andres Gomez <agomez@igalia.com>
---
 src/compiler/glsl/link_varyings.cpp | 23 +++++++++++++++++++++++
 1 file changed, 23 insertions(+)

Patch hide | download patch | download mbox

diff --git a/src/compiler/glsl/link_varyings.cpp b/src/compiler/glsl/link_varyings.cpp
index ab66ceb0d00..28e47f15733 100644
--- a/src/compiler/glsl/link_varyings.cpp
+++ b/src/compiler/glsl/link_varyings.cpp
@@ -1173,6 +1173,29 @@  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 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."
+       */
+      for (unsigned i = 0; i < info->NumOutputs; i++) {
+         const struct gl_transform_feedback_output &output = info->Outputs[i];
+
+         if (output.OutputBuffer != buffer)
+            continue;
+
+         if ((output.DstOffset < xfb_offset + num_components) &&
+             (output.DstOffset + output.NumComponents > xfb_offset)) {
+            linker_error(prog,
+                         "variable '%s', xfb_offset (%d) is causing aliasing.",
+                         this->orig_name, xfb_offset * 4);
+            return false;
+         }
+      }
+
       while (num_components > 0) {
          unsigned output_size = MIN2(num_components, 4 - location_frac);
          assert((info->NumOutputs == 0 && max_outputs == 0) ||

Comments

This causes a O(N^2) behavior in the number of feedback'd variables,
which I suspect is undesirable. Elsewhere we use arrays and/or maps --
since the size of the xfb offset has a max value, an array-based
solution may work well.

On Fri, Feb 1, 2019 at 1:08 PM Andres Gomez <agomez@igalia.com> 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;
>
>     ...
>
>   "
>
> Cc: Timothy Arceri <tarceri@itsqueeze.com>
> Signed-off-by: Andres Gomez <agomez@igalia.com>
> ---
>  src/compiler/glsl/link_varyings.cpp | 23 +++++++++++++++++++++++
>  1 file changed, 23 insertions(+)
>
> diff --git a/src/compiler/glsl/link_varyings.cpp b/src/compiler/glsl/link_varyings.cpp
> index ab66ceb0d00..28e47f15733 100644
> --- a/src/compiler/glsl/link_varyings.cpp
> +++ b/src/compiler/glsl/link_varyings.cpp
> @@ -1173,6 +1173,29 @@ 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 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."
> +       */
> +      for (unsigned i = 0; i < info->NumOutputs; i++) {
> +         const struct gl_transform_feedback_output &output = info->Outputs[i];
> +
> +         if (output.OutputBuffer != buffer)
> +            continue;
> +
> +         if ((output.DstOffset < xfb_offset + num_components) &&
> +             (output.DstOffset + output.NumComponents > xfb_offset)) {
> +            linker_error(prog,
> +                         "variable '%s', xfb_offset (%d) is causing aliasing.",
> +                         this->orig_name, xfb_offset * 4);
> +            return false;
> +         }
> +      }
> +
>        while (num_components > 0) {
>           unsigned output_size = MIN2(num_components, 4 - location_frac);
>           assert((info->NumOutputs == 0 && max_outputs == 0) ||
> --
> 2.20.1
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev