Re: [Mesa-dev] [PATCH 3/3] glsl/linker: validate explicit locations for SSO programs

[Timothy Arceri]

On 20/10/17 22:15, Iago Toral wrote:
On Fri, 2017-10-20 at 13:07 +0200, Iago Toral wrote:
On Fri, 2017-10-20 at 22:03 +1100, Timothy Arceri wrote:

On 20/10/17 21:46, Iago Toral Quiroga wrote:
---
   src/compiler/glsl/link_varyings.cpp | 53
+++++++++++++++++++++++++++++++++++++
   src/compiler/glsl/link_varyings.h   |  4 +++
   src/compiler/glsl/linker.cpp        |  6 +++++
   3 files changed, 63 insertions(+)

diff --git a/src/compiler/glsl/link_varyings.cpp
b/src/compiler/glsl/link_varyings.cpp
index dffb4f98df..d7e407184d 100644
--- a/src/compiler/glsl/link_varyings.cpp
+++ b/src/compiler/glsl/link_varyings.cpp
@@ -599,6 +599,59 @@ validate_explicit_variable_location(struct
gl_context *ctx,
   }
   
   /**
+ * Validate explicit locations for SSO programs. For non-SSO
programs this
+ * is alreadty done in cross_validate_outputs_to_inputs().
+ */
+void
+validate_sso_explicit_locations(struct gl_context *ctx,
+                                struct gl_shader_program *prog)
+{
+   assert(prog->SeparateShader);
+   struct explicit_location_info
explicit_locations_in[MAX_VARYINGS_INCL_PATCH][4];
+   struct explicit_location_info
explicit_locations_out[MAX_VARYINGS_INCL_PATCH][4];
+
+   for (unsigned i = 0; i <= MESA_SHADER_FRAGMENT; i++) {
+      if (prog->_LinkedShaders[i] == NULL)
+         continue;
+
+      gl_linked_shader *sh = prog->_LinkedShaders[i];
+
+      memset(explicit_locations_in, 0,
sizeof(explicit_locations_in));
+      memset(explicit_locations_out, 0,
sizeof(explicit_locations_out));
+
+      foreach_in_list(ir_instruction, node, sh->ir) {
+         ir_variable *const var = node->as_variable();
+
+         if (var == NULL ||
+             !var->data.explicit_location ||
+             var->data.location < VARYING_SLOT_VAR0 ||
+             (var->data.mode != ir_var_shader_in &&
+              var->data.mode != ir_var_shader_out))
+            continue;
+
+         /* Vertex shader inputs and fragment shader outputs are
validated
+          * in assign_attribute_or_color_locations, skip them.
+          */
+         if (sh->Stage == MESA_SHADER_FRAGMENT &&
+             var->data.mode == ir_var_shader_out)
+            continue;
+
+         if (sh->Stage == MESA_SHADER_VERTEX &&
+             var->data.mode == ir_var_shader_in)
+            continue;
+
+         if (!validate_explicit_variable_location(
+               ctx,
+               var->data.mode == ir_var_shader_in ?
explicit_locations_in :
+                                                    explicit_loc
at
ions_out,
+               var, prog, sh)) {
+            return;
+         }
+      }
+   }
+}
+
+/**
    * Validate that outputs from one stage match inputs of another
    */
   void
diff --git a/src/compiler/glsl/link_varyings.h
b/src/compiler/glsl/link_varyings.h
index 081b04ea38..e8486208a2 100644
--- a/src/compiler/glsl/link_varyings.h
+++ b/src/compiler/glsl/link_varyings.h
@@ -300,6 +300,10 @@ link_varyings(struct gl_shader_program
*prog,
unsigned first, unsigned last,
                 struct gl_context *ctx, void *mem_ctx);
   
   void
+validate_sso_explicit_locations(struct gl_context *ctx,
+                                struct gl_shader_program *prog);
+
+void
   cross_validate_outputs_to_inputs(struct gl_context *ctx,
                                    struct gl_shader_program
*prog,
                                    gl_linked_shader *producer,
diff --git a/src/compiler/glsl/linker.cpp
b/src/compiler/glsl/linker.cpp
index 3798309678..cba14f940b 100644
--- a/src/compiler/glsl/linker.cpp
+++ b/src/compiler/glsl/linker.cpp
@@ -4938,6 +4938,12 @@ link_shaders(struct gl_context *ctx,
struct
gl_shader_program *prog)
         prev = i;
      }
   
+   /* Validate location aliasing for SSO programs (this is done
during
+    * cross_validate_outputs_to_inputs for non-SSO programs).
+    */
+   if (prog->SeparateShader)
+      validate_sso_explicit_locations(ctx, prog);

Somewhere in link shaders we already calculate the first and last
shader
stages. I think we should probably pass those to this function and
then
only run the validation on the input of the first and output of
the
last, otherwise we are re-validating things the are already covered
by
the non-sso checks.

Aha, good point, thanks. I'll look into that.

Actually, I think maybe I read your proposal too fast :). The only
place where we check for this for non-sso paths is
cross_validate_outputs_to_inputs() and we don't call that for SSO
programs, so there is no chance that we are re-validating anything
here. Am I missing something?

We do (or at least should) call cross_validate_outputs_to_inputs() for 
SSO programs when they contain more than a single stage in a single 
program. A single SSO program can consist of one or many stages.


Iago

+
      /* Cross-validate uniform blocks between shader stages */
      validate_interstage_uniform_blocks(prog, prog-
_LinkedShaders);

      if (!prog->data->LinkStatus)

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