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

Ilia Mirkin imirkin at alum.mit.edu
Fri Jun 27 10:11:06 PDT 2014


Iago,

Not sure where you are with these patches... I guess some updates have
been requested to rename stream -> index in some places? Anyways, once
the generic bits are all sorted out and reviewed, I'd appreciate it if
they could be pushed out, even if you're still working out hw-related
issues with i965 so that the gallium mesa/st integration work can be
pushed out as well.

Thanks,

  -ilia

On Tue, Jun 24, 2014 at 4:10 AM, Iago Toral <itoral at igalia.com> wrote:
> Hi,
>
> this is a summary of the review process for the multi-stream support series.
> Most of the patches got a reviewed-by by either Chris or Ian and if I am not
> mistaken only 4 patches still need a reviewed-by so I hopefully we get these
> 4 reviewed soon :). Here is the situation of each of these patches:
>
> Patch 11: glsl: Add support for EmitStreamVertex() and EndStreamPrimitive().
> - Ian requested to split this in 3 patches which have already been sent to
> the mailing list as a reply.
>   - glsl: Modify ir_emit_vertex to have a stream.
>   - glsl: Modify ir_end_primitive to have a stream.
>
>   - Add support for EmitStreamVertex() and EndStreamPrimitive().
> - There was some discussion on whether ir_to_mesa.cpp and
> st_glsl_to_tgsi.cpp had to be updated too. Based on what was discussed I
> concluded that it was not necessary in the end.
>
> Patch 12: glsl: Validate vertex emission in geometry shaders.
> - We need to decide if we allow EmitStreamVertex(0) with outputs other than
> points. Since Nvidia allows this Ian asked if AMD allowed this too in order
> to decide if a bug to the spec should be filed. I confirmed that AMD allows
> this too. Maybe we want to test other systems before making a decision
> anyway.
> - I can also change the patch to disallow this behavior until we decide
> otherwise if that is preferred.
>
> Patch 18: i965: Implement GL_PRIMITIVES_GENERATED with non-zero streams.
> - Needs review
>
> Patch 21: docs: mark "Geometry shader multiple streams" as done for i965
> - Needs review
>
> Iago
>
>
> El 2014-06-18 11:51, Iago Toral Quiroga escribió:
>
>> This series addresses review comments of the previous version and adds a
>> few
>> things. Summary of the changes in version 2:
>>
>> Patch 1:
>>   - Ian: multiple layout(location=) qualifiers
>>     + This was not a problem in the end.
>>   - Chris: s/explicitely/explicitly/
>>   - Chris: Remove 'Id' suffix in variables.
>>   - Chris: !this->flags.q.streamId for consistency
>>   - Chris: checks should pass if state->is_version(400, 0)
>>   - Chris: Check against the real limit and not MAX_VERTEX_STREAMS
>>   + Plus fixed some other issues we found
>>
>> Patch 2: already reviewed by Chris, unchanged.
>>
>> Patch 3:
>>   - Chris: Replace magic 4's with appropriate constant
>>     + Also added necessary memset(so_decl, 0, sizeof(so_decl));
>>
>> Patch 4: already reviewed by Chris, unchanged.
>>
>> Patch 5: [DONE]
>>   - Chris: Not right for separable programs.
>>     + Original patch was okay after discussion in the mailing list, but
>>       I modified it to make the code more clear.
>>
>> Patch 6: already reviewed by Chris, unchanged.
>>
>> Patch 7: already reviewed by Chris, unchanged.
>>
>> Patch 8: already reviewed by Chris, unchanged.
>>
>> Patch 9:
>>   - Chris: Does not work with multiple geometry shaders
>>     + Now we only store info in the program and not individual shaders and
>> we
>>       set this value after linking all the shaders together (see patch
>> 12).
>>
>> Patch 10: already reviewed by Chris, unchanged.
>>
>> Patch 11:
>>   - Chris: EmitStreamVertex() and EndStreamPrimitive() should be normal
>> builtins
>>     + Reimplemented this following Chris' recommendations in the mailing
>> list.
>>
>> Patch 12: already reviewed by Chris, but now expanded:
>>   + Includes original patch 12 reviewed-by Chris.
>>   + Sets prog->Geom.UsesStreams (this was being done in patch 11 in the
>> original
>>     series).
>>   + Link error if emitting to wrong streams (this was being done in patch
>> 11 in
>>     original series)
>>
>> Patch 13: already reviewed by Chris, unchanged.
>>
>> Patch 14: already reviewed by Chris, but modified to adapt to changes in
>> the
>> implementation of ir_emit_vertex and ir_end_primitive from patch 11.
>>
>> Patch 15: already reviewed by Chris, unchanged.
>>
>> Patch 16:
>>   - Chris: Do no set Ctx->Const.MaxVertexStreams to MAX_VERTEX_STREAMS
>> globally.
>>     + Now this is done in the i965 driver only (see patch 20).
>>
>> Patch 17: already reviewed by Chris, unchanged.
>>
>> Patch 18: NEW
>>   + Adds multi-stream querying for GL_PRIMITIVES_GENERATED
>>
>> Patch 19 (was patch 18 in original series)
>>   - Chris: s/stream/index
>>   + Plus added the same logic for GL_PRIMITIVES_GENERATED
>>
>> Patch 20: NEW
>>   + Enables up to MAX_VERTEX_STREAMS in i965.
>>
>> Patch 21: NEW
>>   + Mark multiple streams done for i965.
>>
>> Patch 22: NEW
>> Patch 23: NEW
>>   + Fixed incorrect initilization and cloning of UsesEndPrimitive flag.
>>
>>
>> Iago Toral Quiroga (20):
>>   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: Validate vertex emission in geometry shaders.
>>   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.
>>   i965: Implement GL_PRIMITIVES_GENERATED with non-zero streams.
>>   mesa: Enable simultaneous queries on different streams.
>>   i965: Enable vertex streams up to MAX_VERTEX_STREAMS.
>>   mesa: Init Geom.UsesEndPrimitive in shader programs.
>>   mesa: Copy Geom.UsesEndPrimitive when cloning a geometry program.
>>
>> Samuel Iglesias Gonsalvez (3):
>>   glsl: Add parsing support for multi-stream output in geometry shaders.
>>   glsl: include streamId when reading/printing ir_variable IR.
>>   docs: mark "Geometry shader multiple streams" as done for i965
>>
>>  docs/GL3.txt                                      |   2 +-
>>  src/glsl/ast.h                                    |   5 +
>>  src/glsl/ast_to_hir.cpp                           |  17 +++
>>  src/glsl/ast_type.cpp                             |  39 +++++-
>>  src/glsl/builtin_functions.cpp                    |  52 +++++++-
>>  src/glsl/glsl_parser.yy                           |  49 +++++++
>>  src/glsl/glsl_parser_extras.h                     |  18 +++
>>  src/glsl/glsl_types.h                             |   5 +
>>  src/glsl/ir.h                                     |  39 ++++--
>>  src/glsl/ir_hierarchical_visitor.cpp              |  50 +++++---
>>  src/glsl/ir_hierarchical_visitor.h                |   6 +-
>>  src/glsl/ir_hv_accept.cpp                         |  21 ++-
>>  src/glsl/ir_print_visitor.cpp                     |  20 ++-
>>  src/glsl/ir_reader.cpp                            |  28 +++-
>>  src/glsl/ir_rvalue_visitor.cpp                    |  37 ++++++
>>  src/glsl/ir_rvalue_visitor.h                      |   6 +
>>  src/glsl/link_varyings.cpp                        |  41 +++++-
>>  src/glsl/link_varyings.h                          |  17 +++
>>  src/glsl/linker.cpp                               | 148
>> ++++++++++++++++++++--
>>  src/glsl/lower_output_reads.cpp                   |   4 +-
>>  src/glsl/lower_packed_varyings.cpp                |   4 +-
>>  src/glsl/opt_dead_code_local.cpp                  |   2 +-
>>  src/mesa/drivers/dri/i965/brw_context.c           |   4 +
>>  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         |  21 +--
>>  src/mesa/drivers/dri/i965/gen7_sol_state.c        |  67 ++++++----
>>  src/mesa/main/mtypes.h                            |   8 +-
>>  src/mesa/main/queryobj.c                          |  18 +--
>>  src/mesa/main/shaderapi.c                         |   1 +
>>  src/mesa/main/shaderobj.c                         |   2 +
>>  src/mesa/program/program.c                        |   2 +
>>  33 files changed, 681 insertions(+), 113 deletions(-)
>
> _______________________________________________
> 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