[Mesa-dev] [PATCH] glsl: fix consumer_stage restriction to separate shader objects

[Timothy Arceri]

On Fri, 2016-01-15 at 10:04 +0100, Samuel Iglesias Gonsálvez wrote:
> Commit 781d278 did not restrict consumer_stage only to separate
> shader
> objects, which is when we don't know if the consumer stage would be a
> fragment shader added later. In normal programs, when
> consumer_stage == -1, it is because they are not consumed.
> 
> Fixes 4 piglit regressions added by commit 781d278 in radeonsi:
> 
> arb_gpu_shader_fp64-double-gettransformfeedbackvarying
> arb_gpu_shader_fp64-tf-interleaved
> arb_gpu_shader_fp64-tf-interleaved-aligned
> arb_gpu_shader_fp64-tf-separate
> 
> Signed-off-by: Samuel Iglesias Gonsálvez <siglesias at igalia.com>
> Tested-by: Michel Dänzer <michel.daenzer at amd.com>
> ---
>  src/glsl/link_varyings.cpp | 15 +++++++++------
>  1 file changed, 9 insertions(+), 6 deletions(-)
> 
> diff --git a/src/glsl/link_varyings.cpp b/src/glsl/link_varyings.cpp
> index 09f80d0..96f5946 100644
> --- a/src/glsl/link_varyings.cpp
> +++ b/src/glsl/link_varyings.cpp
> @@ -824,7 +824,8 @@ public:
>                     gl_shader_stage producer_stage,
>                     gl_shader_stage consumer_stage);
>     ~varying_matches();
> -   void record(ir_variable *producer_var, ir_variable
> *consumer_var);
> +   void record(ir_variable *producer_var, ir_variable *consumer_var,
> +               bool separate_shader);
>     unsigned assign_locations(struct gl_shader_program *prog,
>                               uint64_t reserved_slots, bool
> separate_shader);
>     void store_locations() const;
> @@ -952,7 +953,8 @@ varying_matches::~varying_matches()
>   * rendering.
>   */
>  void
> -varying_matches::record(ir_variable *producer_var, ir_variable
> *consumer_var)
> +varying_matches::record(ir_variable *producer_var, ir_variable
> *consumer_var,
> +                        bool separate_shader)
>  {
>     assert(producer_var != NULL || consumer_var != NULL);
>  
> @@ -968,7 +970,8 @@ varying_matches::record(ir_variable
> *producer_var, ir_variable *consumer_var)
>     }
>  
>     if ((consumer_var == NULL && producer_var->type
> ->contains_integer()) ||
> -       (consumer_stage != -1 && consumer_stage !=
> MESA_SHADER_FRAGMENT)) {
> +       (consumer_stage != MESA_SHADER_FRAGMENT &&
> +        ((separate_shader && consumer_stage != -1) ||
> !separate_shader))) {


I think you can just make the above line:

(!separate_shader || consumer_stage != -1)


>        /* Since this varying is not being consumed by the fragment
> shader, its
>         * interpolation type varying cannot possibly affect
> rendering.
>         * Also, this variable is non-flat and is (or contains) an
> integer.
> @@ -1667,7 +1670,7 @@ assign_varying_locations(struct gl_context
> *ctx,
>            */
>           if (input_var || (prog->SeparateShader && consumer == NULL)
> ||
>               producer->Type == GL_TESS_CONTROL_SHADER) {
> -            matches.record(output_var, input_var);
> +            matches.record(output_var, input_var, prog
> ->SeparateShader);
>           }
>  
>           /* Only stream 0 outputs can be consumed in the next stage
> */
> @@ -1692,7 +1695,7 @@ assign_varying_locations(struct gl_context
> *ctx,
>               (input_var->data.mode != ir_var_shader_in))
>              continue;
>  
> -         matches.record(NULL, input_var);
> +         matches.record(NULL, input_var, prog->SeparateShader);
>        }
>     }
>  
> @@ -1711,7 +1714,7 @@ assign_varying_locations(struct gl_context
> *ctx,
>        }
>  
>        if (matched_candidate->toplevel_var
> ->data.is_unmatched_generic_inout)
> -         matches.record(matched_candidate->toplevel_var, NULL);
> +         matches.record(matched_candidate->toplevel_var, NULL, prog
> ->SeparateShader);
>     }
>  
>     const uint64_t reserved_slots =