[Mesa-dev] [PATCH 00/18] Multi-stream support for geometry shaders

Chris Forbes chrisf at ijw.co.nz
Wed Jun 11 02:45:25 PDT 2014


I sent comments on patches 1, 3, 5, 9, 11, 16, 18

Patches 2, 4, 6-8, 10, 12-15, 17 are:

Reviewed-by: Chris Forbes <chrisf at ijw.co.nz>

You should also include a patch to docs/GL3.txt marking off this
subfeature for i965 :)


Do you have a bunch of piglits which exercise all the tricky corners
of this? I see a few tests
which require the existence of the stream layout qualifier, but not
covering all the behavior.

-- Chris



On Wed, Jun 11, 2014 at 7:49 PM, Iago Toral Quiroga <itoral at igalia.com> wrote:
> This series brings multi-stream support to geometry shaders as defined by
> ARB_gpu_shader5. It also covers some missing multi-stream functionality from
> ARB_transform_feedback3. This is combined work from Samuel Iglesias and
> myself.
>
> The series includes both required infrastructure in Mesa and specific
> implementation for i965.
>
> Summary:
> Patch 1: GLSL parsing bits.
> Patches 2-8: transform feedback support.
> Patches 9-13: support for vertex/primitive emission to non-zero streams.
> Patches 14-15: ir_reader support.
> Patches 16-18: transform feedback queries support (ARB_transform_feedback3).
>
> Notes:
> I am not very happy with patch 11 but I could not find a better way to do this,
> hopefully someone here knows how to do this better. There is a comment in the
> patch explaining the problem, but here goes a summary:
>
> EmitVertex() is not a builtin-function like most others, when this is
> called we don't really generate code, we simply generate an ir_emit_vertex()
> to mark the point where vertex emission is to be done. Then, this will be
> processed at native code-generation time via
> vec4_gs_visitor::visit(ir_emit_vertex *) (in the i965 driver) to do what this
> means for the specifc hardware we are dealing with.  EmitStreamVertex(n) is
> the same thing, with the exception that it takes a compile-time constant
> parameter with the stream id. The problem we have here is that this input
> variable and its value are not really important until we generate native code,
> it won't be used for anything until that moment, and this makes Mesa think that
> it is unused in the optimization passes for dead code that we run before native
> code genration. As a consequence, these passes will kill the variable and by the
> time we reach vec4_gs_visitor::visit(ir_emit_vertex *) we have lost
> the streamId information.
>
> The way patch 11 works around this problem is to never generate an ir_call for
> EmitStreamVertex(n), instead, when it detects that we are calling this function,
> it evaluates the parameters at that moment (we can do this because by definition
> the parameter must be a compile-time constant expression), and generates the
> ir_emit_vertex with the stream value directly. This is more efficient, gets
> the problem solved and also solves another thing: we want to control if we are
> emitting vertices to streams other than 0 and this also gives as the opportuity
> to detect this situation in a place where we can set state accordingly for
> later use. However, having to create a special case for this in does not
> look very nice. The same goes for EndStreamPrimitive(n).
>
> Iago Toral Quiroga (16):
>   mesa: add StreamId information to transform feedback outputs.
>   i965: Enable transform feedback for streams > 0
>   glsl: Assign GLSL StreamIds to transform feedback outputs.
>   glsl: Fail to link if inter-stage input/outputs are not assigned to
>     stream 0
>   glsl: Add methods to retrive a varying's name and streamId.
>   glsl: Two varyings can't write to the same buffer from different
>     streams.
>   glsl: Only geometry shader outputs can be associated with non-zero
>     streams.
>   glsl: Store info about geometry shaders that emit vertices to non-zero
>     streams.
>   i965/gs: Set number of control data bits for stream mode.
>   glsl: Add support for EmitStreamVertex() and EndStreamPrimitive().
>   glsl: Can't emit vertices to non-zero streams with output !=
>     GL_POINTS.
>   i965/gs: Set control data bits for vertices emitted in stream mode.
>   glsl: include streamId when reading/printing emit-vertex and
>     end-primitive IR.
>   mesa: Include stream information in indexed queries.
>   i965: Implement GL_TRANSFORM_FEEDBACK_PRIMITIVES_WRITTEN with non-zero
>     streams.
>   mesa: Enable simultaneous transform feedback queries on different
>     streams.
>
> Samuel Iglesias Gonsalvez (2):
>   glsl: Add parsing support for multi-stream output in geometry shaders.
>   glsl: include streamId when reading/printing ir_variable IR.
>
>  src/glsl/ast.h                                    |  5 ++
>  src/glsl/ast_function.cpp                         | 37 ++++++++++---
>  src/glsl/ast_to_hir.cpp                           |  6 +++
>  src/glsl/ast_type.cpp                             | 19 ++++++-
>  src/glsl/builtin_functions.cpp                    | 60 +++++++++++++++++++++
>  src/glsl/glsl_parser.yy                           | 45 ++++++++++++++++
>  src/glsl/glsl_parser_extras.cpp                   |  4 ++
>  src/glsl/glsl_parser_extras.h                     |  5 ++
>  src/glsl/ir.h                                     | 23 +++++---
>  src/glsl/ir_print_visitor.cpp                     | 15 +++---
>  src/glsl/ir_reader.cpp                            | 24 +++++++--
>  src/glsl/link_varyings.cpp                        | 40 +++++++++++++-
>  src/glsl/link_varyings.h                          | 17 ++++++
>  src/glsl/linker.cpp                               | 33 ++++++++++++
>  src/mesa/drivers/dri/i965/brw_vec4_gs.c           |  9 ++--
>  src/mesa/drivers/dri/i965/brw_vec4_gs_visitor.cpp | 51 +++++++++++++++++-
>  src/mesa/drivers/dri/i965/brw_vec4_gs_visitor.h   |  1 +
>  src/mesa/drivers/dri/i965/gen6_queryobj.c         |  8 +--
>  src/mesa/drivers/dri/i965/gen7_sol_state.c        | 65 ++++++++++++++---------
>  src/mesa/main/context.c                           |  2 +-
>  src/mesa/main/mtypes.h                            | 10 +++-
>  src/mesa/main/queryobj.c                          | 18 ++++---
>  src/mesa/main/shaderapi.c                         |  1 +
>  src/mesa/main/shaderobj.c                         |  1 +
>  src/mesa/program/program.c                        |  1 +
>  25 files changed, 432 insertions(+), 68 deletions(-)
>
> --
> 1.9.1
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev


More information about the mesa-dev mailing list