On 10/26/2016 04:33 AM, Timothy Arceri wrote:
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-sta...@lists.freedesktop.org
Signed-off-by: Kenneth Graunke <kenn...@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.

Well ... I would not call this a *lie* but rather something like "return the expected size", tell what the API expects the size to be optimization is implementation detail). API expects it to match "implementation-dependent maximum patch size" so that's what we should return, maybe even quote the spec?

series is
Reviewed-by: Tapani Pälli <tapani.pa...@intel.com>


 */

Otherwise series is:

Reviewed-by: Timothy Arceri <timothy.arc...@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);
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Reply via email to