[Mesa-dev] [PATCH 00/18] Multi-stream support for geometry shaders
Jordan Justen
jljusten at gmail.com
Wed Jun 11 09:31:41 PDT 2014
On Wed, Jun 11, 2014 at 2:49 AM, Iago Toral <itoral at igalia.com> wrote:
> Hi Chris,
>
> thanks for the quick review!
>
> On Wed, 2014-06-11 at 21:45 +1200, Chris Forbes wrote:
>> I sent comments on patches 1, 3, 5, 9, 11, 16, 18
>
> We will look into these.
>
>> 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.
>
> No, so far we have been testing this with a standalone program. We will
> check what already exists in piglit and add missing test cases.
This is the only piglit test that I know about:
http://patchwork.freedesktop.org/patch/19224/
Note that it passed on nvidia, but failed on catalyst.
-Jordan
>> 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
>>
>
>
> _______________________________________________
> 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