[Mesa-dev] [PATCH] st/mesa: Don't record garbage streamout information in the non-SSO case.

[Timothy Arceri]

This is much nicer.

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

On 25/10/18 8:16 pm, Kenneth Graunke wrote:
> In the non-SSO case, where multiple shader stages are linked together,
> we were recording garbage pipe_stream_output_info structures for all
> but the last enabled geometry-processing stage.
> 
> Specifically, we were using the gl_transform_feedback_info from
> shader_program->last_vert_prog (the stage whose outputs will be
> recorded)...but were pairing it with the output varying mappings
> from the current shader stage.  For example, a program with a VS and
> GS, the VS's pipe_shader_state would have a pipe_stream_output_info
> based on the GS transform feedback info, but the VS output mapping.
> 
> This generally worked out okay because only the pipe_stream_output_info
> for the last stage really matters - the others can be ignored.  However,
> we'd like to avoid confusing the pipe driver.  In particular, my new
> driver translates the stream out information to hardware packets at
> bind_{vs,tes,gs}_state() time...and was hitting asserts about garbage
> varyings that didn't exist.
> 
> This patch changes st/mesa to record a blank pipe_stream_output_info
> with num_outputs = 0 for all stages prior to last_vert_prog.  The last
> one is captured as normal.
> 
> (In the fully-SSO case, nothing should change - each program contains
> a single shader stage, so last_vert_prog *is* the current shader.)
> 
> Tested with llvmpipe (piglit's gpu profile), and freedreno (a3xx,
> gpu profile with -t transform.feedback).  Fixes several hundred CTS
> tests on my new driver.
> ---
>   src/mesa/state_tracker/st_glsl_to_tgsi.cpp | 20 ++++++--------------
>   src/mesa/state_tracker/st_glsl_to_tgsi.h   |  7 +------
>   src/mesa/state_tracker/st_program.c        | 19 ++++++++-----------
>   3 files changed, 15 insertions(+), 31 deletions(-)
> 
> diff --git a/src/mesa/state_tracker/st_glsl_to_tgsi.cpp b/src/mesa/state_tracker/st_glsl_to_tgsi.cpp
> index dea91c7a189..19bd46d6097 100644
> --- a/src/mesa/state_tracker/st_glsl_to_tgsi.cpp
> +++ b/src/mesa/state_tracker/st_glsl_to_tgsi.cpp
> @@ -7467,25 +7467,17 @@ st_link_shader(struct gl_context *ctx, struct gl_shader_program *prog)
>   }
>   
>   void
> -st_translate_stream_output_info(glsl_to_tgsi_visitor *glsl_to_tgsi,
> -                                const ubyte outputMapping[],
> -                                struct pipe_stream_output_info *so)
> -{
> -   if (!glsl_to_tgsi->shader_program->last_vert_prog)
> -      return;
> -
> -   struct gl_transform_feedback_info *info =
> -      glsl_to_tgsi->shader_program->last_vert_prog->sh.LinkedTransformFeedback;
> -   st_translate_stream_output_info2(info, outputMapping, so);
> -}
> -
> -void
> -st_translate_stream_output_info2(struct gl_transform_feedback_info *info,
> +st_translate_stream_output_info(struct gl_transform_feedback_info *info,
>                                   const ubyte outputMapping[],
>                                   struct pipe_stream_output_info *so)
>   {
>      unsigned i;
>   
> +   if (!info) {
> +      so->num_outputs = 0;
> +      return;
> +   }
> +
>      for (i = 0; i < info->NumOutputs; i++) {
>         so->output[i].register_index =
>            outputMapping[info->Outputs[i].OutputRegister];
> diff --git a/src/mesa/state_tracker/st_glsl_to_tgsi.h b/src/mesa/state_tracker/st_glsl_to_tgsi.h
> index 8ccfff9bd37..fa6c0e02712 100644
> --- a/src/mesa/state_tracker/st_glsl_to_tgsi.h
> +++ b/src/mesa/state_tracker/st_glsl_to_tgsi.h
> @@ -58,12 +58,7 @@ void free_glsl_to_tgsi_visitor(struct glsl_to_tgsi_visitor *v);
>   GLboolean st_link_shader(struct gl_context *ctx, struct gl_shader_program *prog);
>   
>   void
> -st_translate_stream_output_info(struct glsl_to_tgsi_visitor *glsl_to_tgsi,
> -                                const ubyte outputMapping[],
> -                                struct pipe_stream_output_info *so);
> -
> -void
> -st_translate_stream_output_info2(struct gl_transform_feedback_info *info,
> +st_translate_stream_output_info(struct gl_transform_feedback_info *info,
>                                   const ubyte outputMapping[],
>                                   struct pipe_stream_output_info *so);
>   
> diff --git a/src/mesa/state_tracker/st_program.c b/src/mesa/state_tracker/st_program.c
> index af86c47b945..3bc7b0649c4 100644
> --- a/src/mesa/state_tracker/st_program.c
> +++ b/src/mesa/state_tracker/st_program.c
> @@ -458,12 +458,9 @@ st_translate_vertex_program(struct st_context *st,
>      }
>   
>      if (stvp->shader_program) {
> -      struct gl_program *prog = stvp->shader_program->last_vert_prog;
> -      if (prog) {
> -         st_translate_stream_output_info2(prog->sh.LinkedTransformFeedback,
> -                                          stvp->result_to_output,
> -                                          &stvp->tgsi.stream_output);
> -      }
> +      st_translate_stream_output_info(stvp->Base.sh.LinkedTransformFeedback,
> +                                      stvp->result_to_output,
> +                                      &stvp->tgsi.stream_output);
>   
>         st_store_ir_in_disk_cache(st, &stvp->Base, true);
>         return true;
> @@ -505,7 +502,7 @@ st_translate_vertex_program(struct st_context *st,
>                                      output_semantic_name,
>                                      output_semantic_index);
>   
> -      st_translate_stream_output_info(stvp->glsl_to_tgsi,
> +      st_translate_stream_output_info(stvp->Base.sh.LinkedTransformFeedback,
>                                         stvp->result_to_output,
>                                         &stvp->tgsi.stream_output);
>   
> @@ -1417,7 +1414,7 @@ st_translate_program_common(struct st_context *st,
>      }
>      ureg_destroy(ureg);
>   
> -   st_translate_stream_output_info(glsl_to_tgsi,
> +   st_translate_stream_output_info(prog->sh.LinkedTransformFeedback,
>                                      outputMapping,
>                                      &out_state->stream_output);
>   
> @@ -1464,9 +1461,9 @@ st_translate_program_stream_output(struct gl_program *prog,
>         }
>      }
>   
> -   st_translate_stream_output_info2(prog->sh.LinkedTransformFeedback,
> -                                    outputMapping,
> -                                    stream_output);
> +   st_translate_stream_output_info(prog->sh.LinkedTransformFeedback,
> +                                   outputMapping,
> +                                   stream_output);
>   }
>   
>   /**
>