[Mesa-dev] [PATCH 2/3] mesa/st/nir: fix missing nir_compact_varyings

[Timothy Arceri]

On 9/12/18 5:28 am, Rob Clark wrote:
> Not entirely sure when this changed, but it seem like
> LinkedTransformFeedback is (usually?) populated,

Yeah it looks like this code was wrong when introduced. I also recall 
somebody complaining the performance dropped in Shadow of Mordor with 
Eric's fix, which makes a little more sense now.

Looking over the code in link_varying.cpp what happens is we always 
create LinkedTransformFeedback for the last vertex stage. Which means we 
have not been packing the frgament shader inputs since this fix was 
introduced :( Maybe update the commit message to make this a little clearer.

Also please add:

Fixes: dbd52585fa9f ("st/nir: Disable varying packing when doing 
transform feedback.")

With those changes patches 1-2 are:

Reviewed-by: Timothy Arceri <tarceri at itsqueeze.com>

Thanks for looking into this.

  even if
> NumVaryings is zero.  So make the check about whether it
> is safe to nir_compact_varyings() a bit more complete.
> 
> Signed-off-by: Rob Clark <robdclark at gmail.com>
> ---
>   src/mesa/state_tracker/st_glsl_to_nir.cpp | 3 ++-
>   1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/src/mesa/state_tracker/st_glsl_to_nir.cpp b/src/mesa/state_tracker/st_glsl_to_nir.cpp
> index d0475fb538a..7406e26e2f8 100644
> --- a/src/mesa/state_tracker/st_glsl_to_nir.cpp
> +++ b/src/mesa/state_tracker/st_glsl_to_nir.cpp
> @@ -758,7 +758,8 @@ st_link_nir(struct gl_context *ctx,
>             * the pipe_stream_output->output_register field is based on the
>             * pre-compacted driver_locations.
>             */
> -         if (!prev_shader->sh.LinkedTransformFeedback)
> +         if (!(prev_shader->sh.LinkedTransformFeedback &&
> +               prev_shader->sh.LinkedTransformFeedback->NumVarying > 0))
>               nir_compact_varyings(shader_program->_LinkedShaders[prev]->Program->nir,
>                                 nir, ctx->API != API_OPENGL_COMPAT);
>         }
>