[Mesa-dev] [PATCH v2 11/23] glsl: Add support for EmitStreamVertex() and EndStreamPrimitive().
Iago Toral
itoral at igalia.com
Thu Jun 19 03:37:43 PDT 2014
After having a quick look at ir_to_mesa.cpp and st_glsl_to_tgsi.cpp I
have some comments and questions about this:
On Wed, 2014-06-18 at 13:31 -0700, Ian Romanick wrote:
> This patch should be split into several patches:
>
> 1. Modify ir_emit_vertex to have a stream. This patch also needs to update
> ir_to_mesa.cpp and st_glsl_to_tgsi.cpp.
ir_to_mesa.cpp does not currently implement support for emit_vertex and
end_primitive at all:
void
ir_to_mesa_visitor::visit(ir_emit_vertex *)
{
assert(!"Geometry shaders not supported.");
}
void
ir_to_mesa_visitor::visit(ir_end_primitive *)
{
assert(!"Geometry shaders not supported.");
}
so doing this, as far as I can see, involves defining opcodes for
emit_vertex and end_primitive and then handle these opcodes properly in
other places of the code so things can get done I guess. I am not yet
familiar with this parts of the code base, so I guess I'll need some
time to figure how to do this right. Since ir_to_mesa.cpp is not
currently supporting ir_emit_vertex and ir_end_primitive at all, would
it be okay if we do this on a later patch after this series has been
reviewed ad merged?
Also, how can I debug this part of the code? what drivers are using
this?
Regarding st_glsl_to_tgsi.cpp, I think I can use some guidance since I
am not familiar with gallium. Seeing what is being done for other IR
types, I think I need to do something like:
st_src_reg stream_src = ir->stream->accept(this);
and then emit the TGSI_OPCODE_EMIT passing stream_src to it.
However, I see that glsl_to_tgsi_visitor::emit() can take no src/dst
arguments or if it takes, it takes one dst/src pair at least, so I am
not sure what should be done here... what do I have to use for the dst
parameter? should I just pass undef_dst? something else?
> 2. Modify ir_end_primitive to have a stream. This patch also needs to update
> ir_to_mesa.cpp and st_glsl_to_tgsi.cpp.
>
> 3. Add the new built-in functions.
>
> A couple other minor comments below...
>
> On 06/18/2014 02:51 AM, Iago Toral Quiroga wrote:
> > ---
> > src/glsl/builtin_functions.cpp | 52 ++++++++++++++++++++++++++++++++++--
> > src/glsl/ir.h | 34 +++++++++++++++++------
> > src/glsl/ir_hierarchical_visitor.cpp | 50 +++++++++++++++++++++-------------
> > src/glsl/ir_hierarchical_visitor.h | 6 +++--
> > src/glsl/ir_hv_accept.cpp | 21 ++++++++++++---
> > src/glsl/ir_rvalue_visitor.cpp | 37 +++++++++++++++++++++++++
> > src/glsl/ir_rvalue_visitor.h | 6 +++++
> > src/glsl/lower_output_reads.cpp | 4 +--
> > src/glsl/lower_packed_varyings.cpp | 4 +--
> > src/glsl/opt_dead_code_local.cpp | 2 +-
> > 10 files changed, 178 insertions(+), 38 deletions(-)
> >
> > diff --git a/src/glsl/builtin_functions.cpp b/src/glsl/builtin_functions.cpp
> > index f9f0686..07a0722 100644
> > --- a/src/glsl/builtin_functions.cpp
> > +++ b/src/glsl/builtin_functions.cpp
> > @@ -359,6 +359,12 @@ shader_image_load_store(const _mesa_glsl_parse_state *state)
> > state->ARB_shader_image_load_store_enable);
> > }
> >
> > +static bool
> > +gs_streams(const _mesa_glsl_parse_state *state)
> > +{
> > + return gpu_shader5(state) && gs_only(state);
> > +}
> > +
> > /** @} */
> >
> > /******************************************************************************/
> > @@ -594,6 +600,10 @@ private:
> >
> > B0(EmitVertex)
> > B0(EndPrimitive)
> > + ir_function_signature *_EmitStreamVertex(builtin_available_predicate avail,
> > + const glsl_type *stream_type);
> > + ir_function_signature *_EndStreamPrimitive(builtin_available_predicate avail,
> > + const glsl_type *stream_type);
> >
> > B2(textureQueryLod);
> > B1(textureQueryLevels);
> > @@ -1708,6 +1718,14 @@ builtin_builder::create_builtins()
> >
> > add_function("EmitVertex", _EmitVertex(), NULL);
> > add_function("EndPrimitive", _EndPrimitive(), NULL);
> > + add_function("EmitStreamVertex",
> > + _EmitStreamVertex(gs_streams, glsl_type::uint_type),
> > + _EmitStreamVertex(gs_streams, glsl_type::int_type),
> > + NULL);
> > + add_function("EndStreamPrimitive",
> > + _EndStreamPrimitive(gs_streams, glsl_type::uint_type),
> > + _EndStreamPrimitive(gs_streams, glsl_type::int_type),
> > + NULL);
> >
> > add_function("textureQueryLOD",
> > _textureQueryLod(glsl_type::sampler1D_type, glsl_type::float_type),
> > @@ -3872,7 +3890,22 @@ builtin_builder::_EmitVertex()
> > {
> > MAKE_SIG(glsl_type::void_type, gs_only, 0);
> >
> > - body.emit(new(mem_ctx) ir_emit_vertex());
> > + ir_rvalue *stream = new(mem_ctx) ir_constant(0, 1);
> > + body.emit(new(mem_ctx) ir_emit_vertex(stream));
> > +
> > + return sig;
> > +}
> > +
> > +ir_function_signature *
> > +builtin_builder::_EmitStreamVertex(builtin_available_predicate avail,
> > + const glsl_type *stream_type)
> > +{
>
> Please add a spec quotation for this. I had to go look it up to be sure
> ir_var_const_in was correct.
>
> /* Section 8.12 (Geometry Shader Functions) of the OpenGL 4.0 spec says:
> *
> * "Completes the current output primitive on stream stream and starts
> * a new one. The argument to stream must be a constant integral
> * expression."
> */
>
> > + ir_variable *stream =
> > + new(mem_ctx) ir_variable(stream_type, "stream", ir_var_const_in);
> > +
> > + MAKE_SIG(glsl_type::void_type, avail, 1, stream);
> > +
> > + body.emit(new(mem_ctx) ir_emit_vertex(var_ref(stream)));
> >
> > return sig;
> > }
> > @@ -3882,7 +3915,22 @@ builtin_builder::_EndPrimitive()
> > {
> > MAKE_SIG(glsl_type::void_type, gs_only, 0);
> >
> > - body.emit(new(mem_ctx) ir_end_primitive());
> > + ir_rvalue *stream = new(mem_ctx) ir_constant(0, 1);
> > + body.emit(new(mem_ctx) ir_end_primitive(stream));
> > +
> > + return sig;
> > +}
> > +
> > +ir_function_signature *
> > +builtin_builder::_EndStreamPrimitive(builtin_available_predicate avail,
> > + const glsl_type *stream_type)
> > +{
>
> Same spec reference here.
>
> > + ir_variable *stream =
> > + new(mem_ctx) ir_variable(stream_type, "stream", ir_var_const_in);
> > +
> > + MAKE_SIG(glsl_type::void_type, avail, 1, stream);
> > +
> > + body.emit(new(mem_ctx) ir_end_primitive(var_ref(stream)));
> >
> > return sig;
> > }
> > diff --git a/src/glsl/ir.h b/src/glsl/ir.h
> > index dbbabb5..ea5ba27 100644
> > --- a/src/glsl/ir.h
> > +++ b/src/glsl/ir.h
> > @@ -2159,9 +2159,11 @@ private:
> > */
> > class ir_emit_vertex : public ir_instruction {
> > public:
> > - ir_emit_vertex()
> > - : ir_instruction(ir_type_emit_vertex)
> > + ir_emit_vertex(ir_rvalue *stream)
> > + : ir_instruction(ir_type_emit_vertex),
> > + stream(stream)
> > {
> > + assert(stream);
> > }
> >
> > virtual void accept(ir_visitor *v)
> > @@ -2169,12 +2171,19 @@ public:
> > v->visit(this);
> > }
> >
> > - virtual ir_emit_vertex *clone(void *mem_ctx, struct hash_table *) const
> > + virtual ir_emit_vertex *clone(void *mem_ctx, struct hash_table *ht) const
> > {
> > - return new(mem_ctx) ir_emit_vertex();
> > + return new(mem_ctx) ir_emit_vertex(this->stream->clone(mem_ctx, ht));
> > }
> >
> > virtual ir_visitor_status accept(ir_hierarchical_visitor *);
> > +
> > + int stream_id() const
> > + {
> > + return stream->as_constant()->value.i[0];
> > + }
> > +
> > + ir_rvalue *stream;
> > };
> >
> > /**
> > @@ -2183,9 +2192,11 @@ public:
> > */
> > class ir_end_primitive : public ir_instruction {
> > public:
> > - ir_end_primitive()
> > - : ir_instruction(ir_type_end_primitive)
> > + ir_end_primitive(ir_rvalue *stream)
> > + : ir_instruction(ir_type_end_primitive),
> > + stream(stream)
> > {
> > + assert(stream);
> > }
> >
> > virtual void accept(ir_visitor *v)
> > @@ -2193,12 +2204,19 @@ public:
> > v->visit(this);
> > }
> >
> > - virtual ir_end_primitive *clone(void *mem_ctx, struct hash_table *) const
> > + virtual ir_end_primitive *clone(void *mem_ctx, struct hash_table *ht) const
> > {
> > - return new(mem_ctx) ir_end_primitive();
> > + return new(mem_ctx) ir_end_primitive(this->stream->clone(mem_ctx, ht));
> > }
> >
> > virtual ir_visitor_status accept(ir_hierarchical_visitor *);
> > +
> > + int stream_id() const
> > + {
> > + return stream->as_constant()->value.i[0];
> > + }
> > +
> > + ir_rvalue *stream;
> > };
> >
> > /*@}*/
> > diff --git a/src/glsl/ir_hierarchical_visitor.cpp b/src/glsl/ir_hierarchical_visitor.cpp
> > index 2e606dd..d3c00ec 100644
> > --- a/src/glsl/ir_hierarchical_visitor.cpp
> > +++ b/src/glsl/ir_hierarchical_visitor.cpp
> > @@ -69,24 +69,6 @@ ir_hierarchical_visitor::visit(ir_loop_jump *ir)
> > }
> >
> > ir_visitor_status
> > -ir_hierarchical_visitor::visit(ir_emit_vertex *ir)
> > -{
> > - if (this->callback != NULL)
> > - this->callback(ir, this->data);
> > -
> > - return visit_continue;
> > -}
> > -
> > -ir_visitor_status
> > -ir_hierarchical_visitor::visit(ir_end_primitive *ir)
> > -{
> > - if (this->callback != NULL)
> > - this->callback(ir, this->data);
> > -
> > - return visit_continue;
> > -}
> > -
> > -ir_visitor_status
> > ir_hierarchical_visitor::visit(ir_dereference_variable *ir)
> > {
> > if (this->callback != NULL)
> > @@ -303,6 +285,38 @@ ir_hierarchical_visitor::visit_leave(ir_if *ir)
> > return visit_continue;
> > }
> >
> > +ir_visitor_status
> > +ir_hierarchical_visitor::visit_enter(ir_emit_vertex *ir)
> > +{
> > + if (this->callback != NULL)
> > + this->callback(ir, this->data);
> > +
> > + return visit_continue;
> > +}
> > +
> > +ir_visitor_status
> > +ir_hierarchical_visitor::visit_leave(ir_emit_vertex *ir)
> > +{
> > + (void) ir;
> > + return visit_continue;
> > +}
> > +
> > +ir_visitor_status
> > +ir_hierarchical_visitor::visit_enter(ir_end_primitive *ir)
> > +{
> > + if (this->callback != NULL)
> > + this->callback(ir, this->data);
> > +
> > + return visit_continue;
> > +}
> > +
> > +ir_visitor_status
> > +ir_hierarchical_visitor::visit_leave(ir_end_primitive *ir)
> > +{
> > + (void) ir;
> > + return visit_continue;
> > +}
> > +
> > void
> > ir_hierarchical_visitor::run(exec_list *instructions)
> > {
> > diff --git a/src/glsl/ir_hierarchical_visitor.h b/src/glsl/ir_hierarchical_visitor.h
> > index 647d2e0..bc89a04 100644
> > --- a/src/glsl/ir_hierarchical_visitor.h
> > +++ b/src/glsl/ir_hierarchical_visitor.h
> > @@ -87,8 +87,6 @@ public:
> > virtual ir_visitor_status visit(class ir_variable *);
> > virtual ir_visitor_status visit(class ir_constant *);
> > virtual ir_visitor_status visit(class ir_loop_jump *);
> > - virtual ir_visitor_status visit(class ir_emit_vertex *);
> > - virtual ir_visitor_status visit(class ir_end_primitive *);
> >
> > /**
> > * ir_dereference_variable isn't technically a leaf, but it is treated as a
> > @@ -137,6 +135,10 @@ public:
> > virtual ir_visitor_status visit_leave(class ir_discard *);
> > virtual ir_visitor_status visit_enter(class ir_if *);
> > virtual ir_visitor_status visit_leave(class ir_if *);
> > + virtual ir_visitor_status visit_enter(class ir_emit_vertex *);
> > + virtual ir_visitor_status visit_leave(class ir_emit_vertex *);
> > + virtual ir_visitor_status visit_enter(class ir_end_primitive *);
> > + virtual ir_visitor_status visit_leave(class ir_end_primitive *);
> > /*@}*/
> >
> >
> > diff --git a/src/glsl/ir_hv_accept.cpp b/src/glsl/ir_hv_accept.cpp
> > index 2a1f70e..eed761f 100644
> > --- a/src/glsl/ir_hv_accept.cpp
> > +++ b/src/glsl/ir_hv_accept.cpp
> > @@ -405,12 +405,27 @@ ir_if::accept(ir_hierarchical_visitor *v)
> > ir_visitor_status
> > ir_emit_vertex::accept(ir_hierarchical_visitor *v)
> > {
> > - return v->visit(this);
> > -}
> > + ir_visitor_status s = v->visit_enter(this);
> > + if (s != visit_continue)
> > + return (s == visit_continue_with_parent) ? visit_continue : s;
> >
> > + s = this->stream->accept(v);
> > + if (s != visit_continue)
> > + return (s == visit_continue_with_parent) ? visit_continue : s;
> > +
> > + return (s == visit_stop) ? s : v->visit_leave(this);
> > +}
> >
> > ir_visitor_status
> > ir_end_primitive::accept(ir_hierarchical_visitor *v)
> > {
> > - return v->visit(this);
> > + ir_visitor_status s = v->visit_enter(this);
> > + if (s != visit_continue)
> > + return (s == visit_continue_with_parent) ? visit_continue : s;
> > +
> > + s = this->stream->accept(v);
> > + if (s != visit_continue)
> > + return (s == visit_continue_with_parent) ? visit_continue : s;
> > +
> > + return (s == visit_stop) ? s : v->visit_leave(this);
> > }
> > diff --git a/src/glsl/ir_rvalue_visitor.cpp b/src/glsl/ir_rvalue_visitor.cpp
> > index fcbe944..0370170 100644
> > --- a/src/glsl/ir_rvalue_visitor.cpp
> > +++ b/src/glsl/ir_rvalue_visitor.cpp
> > @@ -149,6 +149,19 @@ ir_rvalue_base_visitor::rvalue_visit(ir_if *ir)
> > return visit_continue;
> > }
> >
> > +ir_visitor_status
> > +ir_rvalue_base_visitor::rvalue_visit(ir_emit_vertex *ir)
> > +{
> > + handle_rvalue(&ir->stream);
> > + return visit_continue;
> > +}
> > +
> > +ir_visitor_status
> > +ir_rvalue_base_visitor::rvalue_visit(ir_end_primitive *ir)
> > +{
> > + handle_rvalue(&ir->stream);
> > + return visit_continue;
> > +}
> >
> > ir_visitor_status
> > ir_rvalue_visitor::visit_leave(ir_expression *ir)
> > @@ -205,6 +218,18 @@ ir_rvalue_visitor::visit_leave(ir_if *ir)
> > }
> >
> > ir_visitor_status
> > +ir_rvalue_visitor::visit_leave(ir_emit_vertex *ir)
> > +{
> > + return rvalue_visit(ir);
> > +}
> > +
> > +ir_visitor_status
> > +ir_rvalue_visitor::visit_leave(ir_end_primitive *ir)
> > +{
> > + return rvalue_visit(ir);
> > +}
> > +
> > +ir_visitor_status
> > ir_rvalue_enter_visitor::visit_enter(ir_expression *ir)
> > {
> > return rvalue_visit(ir);
> > @@ -257,3 +282,15 @@ ir_rvalue_enter_visitor::visit_enter(ir_if *ir)
> > {
> > return rvalue_visit(ir);
> > }
> > +
> > +ir_visitor_status
> > +ir_rvalue_enter_visitor::visit_enter(ir_emit_vertex *ir)
> > +{
> > + return rvalue_visit(ir);
> > +}
> > +
> > +ir_visitor_status
> > +ir_rvalue_enter_visitor::visit_enter(ir_end_primitive *ir)
> > +{
> > + return rvalue_visit(ir);
> > +}
> > diff --git a/src/glsl/ir_rvalue_visitor.h b/src/glsl/ir_rvalue_visitor.h
> > index 2179fa5..04ec0fa 100644
> > --- a/src/glsl/ir_rvalue_visitor.h
> > +++ b/src/glsl/ir_rvalue_visitor.h
> > @@ -41,6 +41,8 @@ public:
> > ir_visitor_status rvalue_visit(ir_return *);
> > ir_visitor_status rvalue_visit(ir_swizzle *);
> > ir_visitor_status rvalue_visit(ir_texture *);
> > + ir_visitor_status rvalue_visit(ir_emit_vertex *);
> > + ir_visitor_status rvalue_visit(ir_end_primitive *);
> >
> > virtual void handle_rvalue(ir_rvalue **rvalue) = 0;
> > };
> > @@ -57,6 +59,8 @@ public:
> > virtual ir_visitor_status visit_leave(ir_return *);
> > virtual ir_visitor_status visit_leave(ir_swizzle *);
> > virtual ir_visitor_status visit_leave(ir_texture *);
> > + virtual ir_visitor_status visit_leave(ir_emit_vertex *);
> > + virtual ir_visitor_status visit_leave(ir_end_primitive *);
> > };
> >
> > class ir_rvalue_enter_visitor : public ir_rvalue_base_visitor {
> > @@ -71,4 +75,6 @@ public:
> > virtual ir_visitor_status visit_enter(ir_return *);
> > virtual ir_visitor_status visit_enter(ir_swizzle *);
> > virtual ir_visitor_status visit_enter(ir_texture *);
> > + virtual ir_visitor_status visit_enter(ir_emit_vertex *);
> > + virtual ir_visitor_status visit_enter(ir_end_primitive *);
> > };
> > diff --git a/src/glsl/lower_output_reads.cpp b/src/glsl/lower_output_reads.cpp
> > index afe1776..1ee815d 100644
> > --- a/src/glsl/lower_output_reads.cpp
> > +++ b/src/glsl/lower_output_reads.cpp
> > @@ -52,7 +52,7 @@ public:
> > output_read_remover();
> > ~output_read_remover();
> > virtual ir_visitor_status visit(class ir_dereference_variable *);
> > - virtual ir_visitor_status visit(class ir_emit_vertex *);
> > + virtual ir_visitor_status visit_leave(class ir_emit_vertex *);
> > virtual ir_visitor_status visit_leave(class ir_return *);
> > virtual ir_visitor_status visit_leave(class ir_function_signature *);
> > };
> > @@ -148,7 +148,7 @@ output_read_remover::visit_leave(ir_return *ir)
> > }
> >
> > ir_visitor_status
> > -output_read_remover::visit(ir_emit_vertex *ir)
> > +output_read_remover::visit_leave(ir_emit_vertex *ir)
> > {
> > hash_table_call_foreach(replacements, emit_return_copy, ir);
> > hash_table_clear(replacements);
> > diff --git a/src/glsl/lower_packed_varyings.cpp b/src/glsl/lower_packed_varyings.cpp
> > index e865474..eda56a9 100644
> > --- a/src/glsl/lower_packed_varyings.cpp
> > +++ b/src/glsl/lower_packed_varyings.cpp
> > @@ -613,7 +613,7 @@ public:
> > explicit lower_packed_varyings_gs_splicer(void *mem_ctx,
> > const exec_list *instructions);
> >
> > - virtual ir_visitor_status visit(ir_emit_vertex *ev);
> > + virtual ir_visitor_status visit_leave(ir_emit_vertex *ev);
> >
> > private:
> > /**
> > @@ -637,7 +637,7 @@ lower_packed_varyings_gs_splicer::lower_packed_varyings_gs_splicer(
> >
> >
> > ir_visitor_status
> > -lower_packed_varyings_gs_splicer::visit(ir_emit_vertex *ev)
> > +lower_packed_varyings_gs_splicer::visit_leave(ir_emit_vertex *ev)
> > {
> > foreach_list(node, this->instructions) {
> > ir_instruction *ir = (ir_instruction *) node;
> > diff --git a/src/glsl/opt_dead_code_local.cpp b/src/glsl/opt_dead_code_local.cpp
> > index c27c526..88895fb 100644
> > --- a/src/glsl/opt_dead_code_local.cpp
> > +++ b/src/glsl/opt_dead_code_local.cpp
> > @@ -114,7 +114,7 @@ public:
> > return visit_continue_with_parent;
> > }
> >
> > - virtual ir_visitor_status visit(ir_emit_vertex *)
> > + virtual ir_visitor_status visit_leave(ir_emit_vertex *)
> > {
> > /* For the purpose of dead code elimination, emitting a vertex counts as
> > * "reading" all of the currently assigned output variables.
> >
>
>
More information about the mesa-dev
mailing list