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

Marek Olšák maraeo at gmail.com
Sat Nov 17 03:41:46 UTC 2018


You can try to remove streamout.stride_in_dw and get it directly from the
correct shader where stride_in_dw is used.

Marek

On Wed, Nov 14, 2018 at 12:55 PM Gert Wollny <gw.fossdev at gmail.com> wrote:

> Hi Kenneth,
>
> unfortunately this patch breaks r600. I tried to dig into it to figure
> out where things going wrong but I was not very successful so far.
> While the right shader (TES) registers the stream output in the shader
> translation also with your patch, querying with GL_QUERY_RESULT just
> now returns zero. I've logged my findings in this bug:
>
>   https://bugs.freedesktop.org/show_bug.cgi?id=108734
>
> Any idea where things could go wrong?
>
> Best,
> Gert
>
> Am Donnerstag, den 25.10.2018, 02:16 -0700 schrieb Kenneth Graunke:
> > 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);
> >  }
> >
> >  /**
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20181116/a5c36d6e/attachment-0001.html>


More information about the mesa-dev mailing list