From c81fbb42d94293e78e9c767bb00ad22855f9e0b0 Mon Sep 17 00:00:00 2001 From: Andres Gomez Date: Wed, 30 Jan 2019 15:58:42 +0200 Subject: [PATCH] glsl/linker: check for xfb_offset aliasing MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit 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; ... " Fixes the following piglit test: tests/spec/arb_enhanced_layouts/compiler/transform-feedback-layout-qualifiers/xfb_offset/invalid-overlap.vert Fixes the following test: KHR-GL44.enhanced_layouts.xfb_output_overlapping 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). v4: - Add reference to the fixed piglit test in the commit log. - Add reference to the fixed VK-GL-CTS test in the commit log (Tapani). - Empty initialize the BITSET_WORD pointers array (Tapani). Cc: Timothy Arceri Cc: Ilia Mirkin Signed-off-by: Andres Gomez Reviewed-by: Tapani Pälli --- 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 6f8a87bcd6a..4e00840c53e 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] = {}; 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); -- 2.30.2