On Wed, 2014-06-18 at 13:38 -0700, Ian Romanick wrote: > On 06/18/2014 02:51 AM, Iago Toral Quiroga wrote: > > Check if non-zero streams are used. Fail to link if emitting to unsupported > > streams or emitting to non-zero streams with output type other than > > GL_POINTS. > > --- > > src/glsl/linker.cpp | 148 > > +++++++++++++++++++++++++++++++++++++++++++++++----- > > 1 file changed, 134 insertions(+), 14 deletions(-) > > > > diff --git a/src/glsl/linker.cpp b/src/glsl/linker.cpp > > index 0b6a716..f8ff138 100644 > > --- a/src/glsl/linker.cpp > > +++ b/src/glsl/linker.cpp > > @@ -250,31 +250,100 @@ public: > > } > > }; > > > > - > > /** > > - * Visitor that determines whether or not a shader uses ir_end_primitive. > > + * Visitor that determines the highest stream id to which a (geometry) > > shader > > + * emits vertices. It also checks whether End{Stream}Primitive is ever > > called. > > */ > > -class find_end_primitive_visitor : public ir_hierarchical_visitor { > > +class find_emit_vertex_visitor : public ir_hierarchical_visitor { > > public: > > - find_end_primitive_visitor() > > - : found(false) > > + find_emit_vertex_visitor(int max_allowed) > > + : max_stream_allowed(max_allowed), > > + invalid_stream_id(0), > > + invalid_stream_id_from_emit_vertex(false), > > + end_primitive_found(false), > > + uses_non_zero_stream(false) > > { > > /* empty */ > > } > > > > - virtual ir_visitor_status visit(ir_end_primitive *) > > + virtual ir_visitor_status visit_leave(ir_emit_vertex *ir) > > { > > - found = true; > > - return visit_stop; > > + int stream_id = ir->stream_id(); > > + > > + if (stream_id < 0) { > > + invalid_stream_id = stream_id; > > + invalid_stream_id_from_emit_vertex = true; > > + return visit_stop; > > + } > > + > > + if (stream_id > max_stream_allowed) { > > + invalid_stream_id = stream_id; > > + invalid_stream_id_from_emit_vertex = true; > > + return visit_stop; > > + } > > + > > + if (stream_id != 0) > > + uses_non_zero_stream = true; > > + > > + return visit_continue; > > } > > > > - bool end_primitive_found() > > + virtual ir_visitor_status visit_leave(ir_end_primitive *ir) > > { > > - return found; > > + end_primitive_found = true; > > + > > + int stream_id = ir->stream_id(); > > + > > + if (stream_id < 0) { > > + invalid_stream_id = stream_id; > > + invalid_stream_id_from_emit_vertex = false; > > + return visit_stop; > > + } > > + > > + if (stream_id > max_stream_allowed) { > > + invalid_stream_id = stream_id; > > + invalid_stream_id_from_emit_vertex = false; > > + return visit_stop; > > + } > > + > > + if (stream_id != 0) > > + uses_non_zero_stream = true; > > + > > + return visit_continue; > > + } > > + > > + bool error() > > + { > > + return invalid_stream_id != 0; > > + } > > + > > + const char *error_func() > > + { > > + return invalid_stream_id_from_emit_vertex ? > > + "EmitStreamVertex" : "EndStreamPrimitive"; > > + } > > + > > + int error_stream() > > + { > > + return invalid_stream_id; > > + } > > + > > + bool uses_streams() > > + { > > + return uses_non_zero_stream; > > + } > > + > > + bool uses_end_primitive() > > + { > > + return end_primitive_found; > > } > > > > private: > > - bool found; > > + int max_stream_allowed; > > + int invalid_stream_id; > > + bool invalid_stream_id_from_emit_vertex; > > + bool end_primitive_found; > > + bool uses_non_zero_stream; > > }; > > > > } /* anonymous namespace */ > > @@ -551,10 +620,58 @@ validate_geometry_shader_executable(struct > > gl_shader_program *prog, > > > > analyze_clip_usage(prog, shader, &prog->Geom.UsesClipDistance, > > &prog->Geom.ClipDistanceArraySize); > > +} > > + > > +/** > > + * Check if geometry shaders emit to non-zero streams and do corresponding > > + * validations. > > + */ > > +static void > > +validate_geometry_shader_emissions(struct gl_context *ctx, > > + struct gl_shader_program *prog) > > +{ > > + if (prog->_LinkedShaders[MESA_SHADER_GEOMETRY] != NULL) { > > + find_emit_vertex_visitor emit_vertex(ctx->Const.MaxVertexStreams - > > 1); > > + emit_vertex.run(prog->_LinkedShaders[MESA_SHADER_GEOMETRY]->ir); > > + if (emit_vertex.error()) { > > + linker_error(prog, "Invalid call %s(%d). Accepted values for the " > > + "stream parameter are in the range [0, %d].", > > + emit_vertex.error_func(), > > + emit_vertex.error_stream(), > > + ctx->Const.MaxVertexStreams - 1); > > + } > > + prog->Geom.UsesStreams = emit_vertex.uses_streams(); > > + prog->Geom.UsesEndPrimitive = emit_vertex.uses_end_primitive(); > > > > - find_end_primitive_visitor end_primitive; > > - end_primitive.run(shader->ir); > > - prog->Geom.UsesEndPrimitive = end_primitive.end_primitive_found(); > > + /* From the ARB_gpu_shader5 spec: > > + * > > + * "Multiple vertex streams are supported only if the output > > primitive > > + * type is declared to be "points". A program will fail to link > > if it > > + * contains a geometry shader calling EmitStreamVertex() or > > + * EndStreamPrimitive() if its output primitive type is not > > "points". > > + * > > + * However, in the same spec: > > + * > > + * "The function EmitVertex() is equivalent to calling > > EmitStreamVertex() > > + * with <stream> set to zero." > > + * > > + * And: > > + * > > + * "The function EndPrimitive() is equivalent to calling > > + * EndStreamPrimitive() with <stream> set to zero." > > + * > > + * Since we can call EmitVertex() and EndPrimitive() when we output > > + * primitives other than points, calling EmitStreamVertex(0) or > > + * EmitEndPrimitive(0) should not produce errors. This it also what > > Nvidia > > + * does. Currently we only set prog->Geom.UsesStreams to TRUE when > > + * EmitStreamVertex() or EmitEndPrimitive() are called with a > > non-zero > > + * stream. > > Does AMD also behave this way? If so, I can submit a spec bug to make > these explicitly legal. Otherwise, I think we should not allow it. We > probably ought to check Apple and the Intel Windows driver too...
I am not sure that I have access to all of these systems, but let me see. FWIW, the spec also says this: "The function EmitVertex() is equivalent to calling EmitStreamVertex() with <stream> set to zero." And they can't be equivalent if ones goes through checks that the other does not... > > + */ > > + if (prog->Geom.UsesStreams && prog->Geom.OutputType != GL_POINTS) { > > + linker_error(prog, "EmitStreamVertex(n) and EndStreamPrimitive(n) > > " > > + "with n>0 requires point output"); > > + } > > + } > > } > > > > > > @@ -2556,6 +2673,9 @@ link_shaders(struct gl_context *ctx, struct > > gl_shader_program *prog) > > ; > > } > > > > + /* Check and validate stream emissions in geometry shaders */ > > + validate_geometry_shader_emissions(ctx, prog); > > + > > /* Mark all generic shader inputs and outputs as unpaired. */ > > for (unsigned i = MESA_SHADER_VERTEX; i <= MESA_SHADER_FRAGMENT; i++) { > > if (prog->_LinkedShaders[i] != NULL) { > > > > _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev