[Mesa-dev] [PATCH v3 6/6] glsl/linker: check for xfb_offset aliasing
Tapani Pälli
tapani.palli at intel.com
Tue Apr 23 07:00:39 UTC 2019
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 <tarceri at itsqueeze.com>
> Cc: Ilia Mirkin <imirkin at alum.mit.edu>
> Signed-off-by: Andres Gomez <agomez at 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.palli at 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);
>
>
More information about the mesa-dev
mailing list