[Mesa-dev] [PATCH 3/7] glsl: optimise inputs/outputs with explicit locations
gregory hainaut
gregory.hainaut at gmail.com
Sat Nov 21 07:50:10 PST 2015
Hello Timoty,
The patch introduces a regression on deqp test. (I'm nearly sure there are hidden piglit tests that trigger the same error)
DEQP test: deqp-gles31 --deqp-case=dEQP-GLES31.functional.separate_shader.interface.same_location
Be awware that you need to patch Mesa (temporary hack) to start the test. See below
The relevant log:
// vertex interface
layout(location=0) out highp float vtxVar0;
// fragment interface
layout(location=0) out highp float frgVar0;
// mesa warning
warning: fragment shader varying frgVar0 not written by vertex shader
Cheers,
Gregory
diff --git a/src/mesa/main/pipelineobj.c b/src/mesa/main/pipelineobj.c
index 90dff13..6e333c5 100644
--- a/src/mesa/main/pipelineobj.c
+++ b/src/mesa/main/pipelineobj.c
@@ -646,7 +646,8 @@ _mesa_GetProgramPipelineiv(GLuint pipeline, GLenum pname, GLint *params)
return;
case GL_VALIDATE_STATUS:
/* If pipeline is not bound, return initial value 0. */
- *params = (ctx->_Shader->Name != pipe->Name) ? 0 : pipe->Validated;
+ //*params = (ctx->_Shader->Name != pipe->Name) ? 0 : pipe->Validated;
+ *params = pipe->Validated;
return;
case GL_VERTEX_SHADER:
*params = pipe->CurrentProgram[MESA_SHADER_VERTEX]
On Sat, 21 Nov 2015 19:02:02 +1100
Timothy Arceri <t_arceri at yahoo.com.au> wrote:
> From: Timothy Arceri <timothy.arceri at collabora.com>
>
> This change allows used defined inputs/outputs with explicit locations
> to be removed if they are detected to not be used between shaders
> at link time.
>
> To enable this we change the is_unmatched_generic_inout field to be
> flagged when we have a user defined varying. Previously
> explicit_location was assumed to be set only in builtins however SSO
> allows the user to set an explicit location.
>
> We then add a function to match explicit locations between shaders.
>
> Cc: Gregory Hainaut <gregory.hainaut at gmail.com>
> ---
> src/glsl/link_varyings.cpp | 6 ++--
> src/glsl/linker.cpp | 82 +++++++++++++++++++++++++++++++++++++++-------
> 2 files changed, 74 insertions(+), 14 deletions(-)
>
> diff --git a/src/glsl/link_varyings.cpp b/src/glsl/link_varyings.cpp
> index c0b4b3e..ac2755f 100644
> --- a/src/glsl/link_varyings.cpp
> +++ b/src/glsl/link_varyings.cpp
> @@ -896,8 +896,10 @@ varying_matches::record(ir_variable *producer_var, ir_variable *consumer_var)
> {
> assert(producer_var != NULL || consumer_var != NULL);
>
> - if ((producer_var && !producer_var->data.is_unmatched_generic_inout)
> - || (consumer_var && !consumer_var->data.is_unmatched_generic_inout)) {
> + if ((producer_var && (!producer_var->data.is_unmatched_generic_inout ||
> + producer_var->data.explicit_location)) ||
> + (consumer_var && (!consumer_var->data.is_unmatched_generic_inout ||
> + consumer_var->data.explicit_location))) {
> /* Either a location already exists for this variable (since it is part
> * of fixed functionality), or it has already been recorded as part of a
> * previous match.
> diff --git a/src/glsl/linker.cpp b/src/glsl/linker.cpp
> index 5ff433c..1db7b7a 100644
> --- a/src/glsl/linker.cpp
> +++ b/src/glsl/linker.cpp
> @@ -631,20 +631,12 @@ link_invalidate_variable_locations(exec_list *ir)
>
> /* ir_variable::is_unmatched_generic_inout is used by the linker while
> * connecting outputs from one stage to inputs of the next stage.
> - *
> - * There are two implicit assumptions here. First, we assume that any
> - * built-in variable (i.e., non-generic in or out) will have
> - * explicit_location set. Second, we assume that any generic in or out
> - * will not have explicit_location set.
> - *
> - * This second assumption will only be valid until
> - * GL_ARB_separate_shader_objects is supported. When that extension is
> - * implemented, this function will need some modifications.
> */
> - if (!var->data.explicit_location) {
> - var->data.is_unmatched_generic_inout = 1;
> - } else {
> + if (var->data.explicit_location &&
> + var->data.location < VARYING_SLOT_VAR0) {
> var->data.is_unmatched_generic_inout = 0;
> + } else {
> + var->data.is_unmatched_generic_inout = 1;
> }
> }
> }
> @@ -2421,6 +2413,7 @@ assign_attribute_or_color_locations(gl_shader_program *prog,
> continue;
>
> if (var->data.explicit_location) {
> + var->data.is_unmatched_generic_inout = 0;
> if ((var->data.location >= (int)(max_index + generic_base))
> || (var->data.location < 0)) {
> linker_error(prog,
> @@ -2690,6 +2683,61 @@ assign_attribute_or_color_locations(gl_shader_program *prog,
> return true;
> }
>
> +/**
> + * Match explicit locations of outputs to inputs and deactivate the
> + * unmatch flag if found so we don't optimise them alway.
> + */
> +void
> +match_explicit_outputs_to_inputs(struct gl_shader_program *prog,
> + gl_shader *producer,
> + gl_shader *consumer)
> +{
> + glsl_symbol_table parameters;
> + ir_variable *explicit_locations[MAX_VARYING] = { NULL, };
> +
> + /* Find all shader outputs in the "producer" stage.
> + */
> + foreach_in_list(ir_instruction, node, producer->ir) {
> + ir_variable *const var = node->as_variable();
> +
> + if ((var == NULL) || (var->data.mode != ir_var_shader_out))
> + continue;
> +
> + /* Mark output as matched if separte shader with no linked consumer */
> + if (consumer == NULL)
> + var->data.is_unmatched_generic_inout = 0;
> +
> + if (var->data.explicit_location &&
> + var->data.location >= VARYING_SLOT_VAR0) {
> + const unsigned idx = var->data.location - VARYING_SLOT_VAR0;
> + if (explicit_locations[idx] == NULL)
> + explicit_locations[idx] = var;
> + }
> + }
> +
> + /* Match inputs to outputs */
> + foreach_in_list(ir_instruction, node, consumer->ir) {
> + ir_variable *const input = node->as_variable();
> +
> + if ((input == NULL) || (input->data.mode != ir_var_shader_in))
> + continue;
> +
> + /* Mark input as matched if separte shader with no linked producer */
> + if (producer == NULL)
> + input->data.is_unmatched_generic_inout = 0;
> +
> + ir_variable *output = NULL;
> + if (input->data.explicit_location
> + && input->data.location >= VARYING_SLOT_VAR0) {
> + output = explicit_locations[input->data.location - VARYING_SLOT_VAR0];
> +
> + if (output != NULL){
> + input->data.is_unmatched_generic_inout = 0;
> + output->data.is_unmatched_generic_inout = 0;
> + }
> + }
> + }
> +}
>
> /**
> * Demote shader inputs and outputs that are not used in other stages
> @@ -4238,6 +4286,16 @@ link_shaders(struct gl_context *ctx, struct gl_shader_program *prog)
> lower_const_arrays_to_uniforms(prog->_LinkedShaders[i]->ir);
> }
>
> + prev = first;
> + for (unsigned i = prev + 1; i <= MESA_SHADER_FRAGMENT; i++) {
> + if (prog->_LinkedShaders[i] == NULL)
> + continue;
> +
> + match_explicit_outputs_to_inputs(prog, prog->_LinkedShaders[prev],
> + prog->_LinkedShaders[i]);
> + prev = i;
> + }
> +
> /* Validation for special cases where we allow sampler array indexing
> * with loop induction variable. This check emits a warning or error
> * depending if backend can handle dynamic indexing.
More information about the mesa-dev
mailing list