[Mesa-dev] [PATCH 2/2] glsl: Size TCS->TES unsized arrays to gl_MaxPatchVertices for queries.
Timothy Arceri
timothy.arceri at collabora.com
Wed Oct 26 01:33:13 UTC 2016
On Tue, 2016-10-25 at 13:15 -0700, Kenneth Graunke wrote:
> SSO validation and other program interface queries want to see that
> unsized (non-patch) TCS output/TES input arrays are implicitly sized
> to gl_MaxPatchVertices.
>
> By the time we create the program resource lists, we've sized the
> arrays
> to their actual size. (We try to create TCS output arrays to match
> the
> output patch size right away, and at this point, we should have
> shrunk
> TES input arrays.) One option would be to keep them sized to
> gl_MaxPatchVertices, and defer shrinking them. But that's a big
> change,
> and I don't think it's a good idea.
>
> Instead, this patch introduces a new ir_variable flag which indicates
> the variable is implicitly to gl_MaxPatchVertices. Then, the linker
> munges the types when creating the resource list, ignoring the size
> in the IR's types. Basically, lie about it for resource queries.
> It's ugly, but I think it ought to work.
>
> We probably could use var->data.implicit_sized_array for this, but
> I opted for a separate bit to try and avoid convoluting the existing
> SSBO handling. They're similar in concept, but share none of the
> same code...
>
> Fixes:
> ES31-
> CTS.core.tessellation_shader.single.xfb_captures_data_from_correct_st
> age
> and the ES32-CTS and ESEXT-CTS variants.
>
> Cc: mesa-stable at lists.freedesktop.org
> Signed-off-by: Kenneth Graunke <kenneth at whitecape.org>
> ---
> src/compiler/glsl/ast_to_hir.cpp | 3 +++
> src/compiler/glsl/ir.h | 6 ++++++
> src/compiler/glsl/linker.cpp | 24
> +++++++++++++++++++---
> src/compiler/glsl/lower_named_interface_blocks.cpp | 2 ++
> 4 files changed, 32 insertions(+), 3 deletions(-)
>
> diff --git a/src/compiler/glsl/ast_to_hir.cpp
> b/src/compiler/glsl/ast_to_hir.cpp
> index adedcbb..a9a1ba3 100644
> --- a/src/compiler/glsl/ast_to_hir.cpp
> +++ b/src/compiler/glsl/ast_to_hir.cpp
> @@ -4330,6 +4330,8 @@ handle_tess_ctrl_shader_output_decl(struct
> _mesa_glsl_parse_state *state,
> if (var->data.patch)
> return;
>
> + var->data.tess_varying_implicit_sized_array = var->type-
> >is_unsized_array();
> +
> validate_layout_qualifier_vertex_count(state, loc, var,
> num_vertices,
> &state->tcs_output_size,
> "tessellation control
> shader output");
> @@ -4366,6 +4368,7 @@ handle_tess_shader_input_decl(struct
> _mesa_glsl_parse_state *state,
> if (var->type->is_unsized_array()) {
> var->type = glsl_type::get_array_instance(var->type-
> >fields.array,
> state->Const.MaxPatchVertices);
> + var->data.tess_varying_implicit_sized_array = true;
> } else if (var->type->length != state->Const.MaxPatchVertices) {
> _mesa_glsl_error(&loc, state,
> "per-vertex tessellation shader input arrays
> must be "
> diff --git a/src/compiler/glsl/ir.h b/src/compiler/glsl/ir.h
> index 3d28dd5..f07e3b2 100644
> --- a/src/compiler/glsl/ir.h
> +++ b/src/compiler/glsl/ir.h
> @@ -833,6 +833,12 @@ public:
> unsigned implicit_sized_array:1;
>
> /**
> + * Is this a non-patch TCS output / TES input array that was
> implicitly
> + * sized to gl_MaxPatchVertices?
> + */
> + unsigned tess_varying_implicit_sized_array:1;
> +
> + /**
> * Whether this is a fragment shader output implicitly
> initialized with
> * the previous contents of the specified render target at the
> * framebuffer location corresponding to this shader
> invocation.
> diff --git a/src/compiler/glsl/linker.cpp
> b/src/compiler/glsl/linker.cpp
> index 116e17b5..45a6c97 100644
> --- a/src/compiler/glsl/linker.cpp
> +++ b/src/compiler/glsl/linker.cpp
> @@ -3584,6 +3584,7 @@ static gl_shader_variable *
> create_shader_variable(struct gl_shader_program *shProg,
> const ir_variable *in,
> const char *name, const glsl_type *type,
> + const glsl_type *interface_type,
> bool use_implicit_location, int location,
> const glsl_type *outermost_struct_type)
> {
> @@ -3641,7 +3642,7 @@ create_shader_variable(struct gl_shader_program
> *shProg,
>
> out->type = type;
> out->outermost_struct_type = outermost_struct_type;
> - out->interface_type = in->get_interface_type();
> + out->interface_type = interface_type;
> out->component = in->data.location_frac;
> out->index = in->data.index;
> out->patch = in->data.patch;
> @@ -3653,6 +3654,17 @@ create_shader_variable(struct
> gl_shader_program *shProg,
> return out;
> }
>
> +static const glsl_type *
> +resize_to_max_patch_vertices(const struct gl_context *ctx,
> + const glsl_type *type)
> +{
> + if (!type)
> + return NULL;
> +
> + return glsl_type::get_array_instance(type->fields.array,
> + ctx-
> >Const.MaxPatchVertices);
> +}
> +
> static bool
> add_shader_variable(const struct gl_context *ctx,
> struct gl_shader_program *shProg,
> @@ -3699,6 +3711,12 @@ add_shader_variable(const struct gl_context
> *ctx,
> }
>
> default: {
It sucks that we need to do this :(
Since this is a hack and not imediatly obvious as to why we do it. It
would be good to add a comment here. Maybe something like:
/* SSO validation and other program interface queries want to see that
* unsized (non-patch) TCS output/TES input arrays are implicitly sized
* to gl_MaxPatchVertices.
* By the time we get here, we've shrunk the arrays the to their actual
* size. So we create new types here to lie about it for resource
* queries.
*/
Otherwise series is:
Reviewed-by: Timothy Arceri <timothy.arceri at collabora.com>
> + const glsl_type *interface_type = var->get_interface_type();
> + if (var->data.tess_varying_implicit_sized_array) {
> + type = resize_to_max_patch_vertices(ctx, type);
> + interface_type = resize_to_max_patch_vertices(ctx,
> interface_type);
> + }
> +
> /* Issue #16 of the ARB_program_interface_query spec says:
> *
> * "* If a variable is a member of an interface block without
> an
> @@ -3711,8 +3729,7 @@ add_shader_variable(const struct gl_context
> *ctx,
> */
> const char *prefixed_name = (var->data.from_named_ifc_block &&
> !is_gl_identifier(var->name))
> - ? ralloc_asprintf(shProg, "%s.%s", var-
> >get_interface_type()->name,
> - name)
> + ? ralloc_asprintf(shProg, "%s.%s", interface_type->name,
> name)
> : name;
>
> /* The ARB_program_interface_query spec says:
> @@ -3723,6 +3740,7 @@ add_shader_variable(const struct gl_context
> *ctx,
> */
> gl_shader_variable *sha_v =
> create_shader_variable(shProg, var, prefixed_name, type,
> + interface_type,
> use_implicit_location, location,
> outermost_struct_type);
> if (!sha_v)
> diff --git a/src/compiler/glsl/lower_named_interface_blocks.cpp
> b/src/compiler/glsl/lower_named_interface_blocks.cpp
> index a00e60d..53ef638 100644
> --- a/src/compiler/glsl/lower_named_interface_blocks.cpp
> +++ b/src/compiler/glsl/lower_named_interface_blocks.cpp
> @@ -193,6 +193,8 @@
> flatten_named_interface_blocks_declarations::run(exec_list
> *instructions)
> new_var->data.patch = iface_t-
> >fields.structure[i].patch;
> new_var->data.stream = var->data.stream;
> new_var->data.how_declared = var->data.how_declared;
> + new_var->data.tess_varying_implicit_sized_array =
> + var->data.tess_varying_implicit_sized_array;
> new_var->data.from_named_ifc_block = 1;
>
> new_var->init_interface_type(var->type);
More information about the mesa-dev
mailing list