Skip to content

Commit

Permalink
glsl/linker: check for xfb_offset aliasing
Browse files Browse the repository at this point in the history 
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 <[email protected]>
Cc: Ilia Mirkin <[email protected]>
Signed-off-by: Andres Gomez <[email protected]>
Reviewed-by: Tapani Pälli <[email protected]>
  • Loading branch information
@tanty
tanty committed Apr 29, 2019
1 parent 812288b commit c81fbb4
Show file tree
Hide file tree
Showing 2 changed files with 84 additions and 31 deletions.
109 changes: 80 additions & 29 deletions src/compiler/glsl/link_varyings.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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) ||
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -1431,14 +1478,16 @@ 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 */
for (unsigned i = 0; i < num_tfeedback_decls; ++i) {
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;
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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;
}
}
Expand Down Expand Up @@ -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;
Expand Down
6 changes: 4 additions & 2 deletions src/compiler/glsl/link_varyings.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@

#include "main/glheader.h"
#include "program/prog_parameter.h"

#include "util/bitset.h"

struct gl_shader_program;
struct gl_shader;
Expand Down Expand Up @@ -99,7 +99,9 @@ class tfeedback_decl
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);

Expand Down

0 comments on commit c81fbb4

Please sign in to comment.