Re: [Mesa-dev] [PATCH v3 6/6] glsl/linker: check for xfb_offset aliasing
On Tue, 2019-04-23 at 10:00 +0300, Tapani Pälli wrote: > Remember to add: > > --- 8< --- > Fixes the following test: > KHR-GL44.enhanced_layouts.xfb_output_overlapping > --- 8< --- Good point. I developed this patch when this test was yet not "corrected" and failing for us. > > > 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 > > Cc: Ilia Mirkin > > Signed-off-by: Andres Gomez > > --- > > 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++)
Re: [Mesa-dev] [PATCH v3 6/6] glsl/linker: check for xfb_offset aliasing
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 Cc: Ilia Mirkin Signed-off-by: Andres Gomez --- 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); + } +
[Mesa-dev] [PATCH v3 6/6] glsl/linker: check for xfb_offset aliasing
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 Cc: Ilia Mirkin Signed-off-by: Andres Gomez --- 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