From 138804fdfce719ccb1e1f5b0a448e4e19c414bdc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marek=20Ol=C5=A1=C3=A1k?= Date: Sun, 7 Jan 2024 13:43:26 -0500 Subject: [PATCH] glthread: add no_error variants of glDrawElements* The main motivation is that no_error allows us to drop count==0 draws at the beginning of the marshal function, instead of forwarding them to the frontend thread. Such draws are plentiful with Viewperf. Acked-by: Pierre-Eric Pelloux-Prayer Part-of: --- src/mapi/glapi/gen/ARB_base_instance.xml | 5 +- .../gen/ARB_draw_elements_base_vertex.xml | 8 +- src/mapi/glapi/gen/ARB_draw_instanced.xml | 2 +- src/mapi/glapi/gen/gl_API.dtd | 3 + src/mapi/glapi/gen/gl_API.xml | 6 +- src/mapi/glapi/gen/gl_marshal.py | 12 +- src/mapi/glapi/gen/gl_marshal_h.py | 2 + src/mapi/glapi/gen/marshal_XML.py | 1 + src/mesa/main/glthread_draw.c | 125 +++++++++++++++--- 9 files changed, 135 insertions(+), 29 deletions(-) diff --git a/src/mapi/glapi/gen/ARB_base_instance.xml b/src/mapi/glapi/gen/ARB_base_instance.xml index 5dbf895a4dd..d6c17c155b9 100644 --- a/src/mapi/glapi/gen/ARB_base_instance.xml +++ b/src/mapi/glapi/gen/ARB_base_instance.xml @@ -17,7 +17,8 @@ - + @@ -27,7 +28,7 @@ + marshal_struct="public" marshal_no_error="true"> diff --git a/src/mapi/glapi/gen/ARB_draw_elements_base_vertex.xml b/src/mapi/glapi/gen/ARB_draw_elements_base_vertex.xml index 736eda82e4a..a7353c544ff 100644 --- a/src/mapi/glapi/gen/ARB_draw_elements_base_vertex.xml +++ b/src/mapi/glapi/gen/ARB_draw_elements_base_vertex.xml @@ -9,7 +9,7 @@ + marshal_struct="public" marshal_no_error="true"> @@ -17,7 +17,8 @@ - + @@ -36,7 +37,8 @@ - + diff --git a/src/mapi/glapi/gen/ARB_draw_instanced.xml b/src/mapi/glapi/gen/ARB_draw_instanced.xml index 9ff2a56cc0a..5418fd62917 100644 --- a/src/mapi/glapi/gen/ARB_draw_instanced.xml +++ b/src/mapi/glapi/gen/ARB_draw_instanced.xml @@ -16,7 +16,7 @@ + marshal_struct="public" marshal_no_error="true"> diff --git a/src/mapi/glapi/gen/gl_API.dtd b/src/mapi/glapi/gen/gl_API.dtd index a5aae816c48..0358dc9609d 100644 --- a/src/mapi/glapi/gen/gl_API.dtd +++ b/src/mapi/glapi/gen/gl_API.dtd @@ -45,6 +45,7 @@ marshal_call_before CDATA #IMPLIED> marshal_call_after CDATA #IMPLIED> marshal_struct CDATA #IMPLIED> + marshal_no_error CDATA #IMPLIED> @@ -143,6 +144,8 @@ param: header file instead of the C file. It's done even with marshal="custom", in which case you don't have to define the structure manually. + marshal_no_error - indicate that a no_error marshal function will be + generated, only useful with marshal="custom" glx: rop - Opcode value for "render" commands diff --git a/src/mapi/glapi/gen/gl_API.xml b/src/mapi/glapi/gen/gl_API.xml index 91549c214cb..6f5d68f6cf3 100644 --- a/src/mapi/glapi/gen/gl_API.xml +++ b/src/mapi/glapi/gen/gl_API.xml @@ -3218,7 +3218,8 @@ - + @@ -3791,7 +3792,8 @@ - + diff --git a/src/mapi/glapi/gen/gl_marshal.py b/src/mapi/glapi/gen/gl_marshal.py index d571f047532..233a60d5db0 100644 --- a/src/mapi/glapi/gen/gl_marshal.py +++ b/src/mapi/glapi/gen/gl_marshal.py @@ -279,8 +279,16 @@ class PrintCode(gl_XML.gl_print_base): if not condition: continue - settings_by_condition[condition].append( - 'SET_{0}(table, _mesa_marshal_{0});'.format(func.name)) + if func.marshal_no_error: + no_error_condition = '_mesa_is_no_error_enabled(ctx) && ({0})'.format(condition) + error_condition = '!_mesa_is_no_error_enabled(ctx) && ({0})'.format(condition) + settings_by_condition[no_error_condition].append( + 'SET_{0}(table, _mesa_marshal_{0}_no_error);'.format(func.name)) + settings_by_condition[error_condition].append( + 'SET_{0}(table, _mesa_marshal_{0});'.format(func.name)) + else: + settings_by_condition[condition].append( + 'SET_{0}(table, _mesa_marshal_{0});'.format(func.name)) # Print out an if statement for each unique condition, with # the SET_* calls nested inside it. diff --git a/src/mapi/glapi/gen/gl_marshal_h.py b/src/mapi/glapi/gen/gl_marshal_h.py index d155d233340..4dd8acc3dfd 100644 --- a/src/mapi/glapi/gen/gl_marshal_h.py +++ b/src/mapi/glapi/gen/gl_marshal_h.py @@ -76,6 +76,8 @@ class PrintCode(gl_XML.gl_print_base): if flavor in ('custom', 'async', 'sync') and not func.marshal_is_static(): print('{0} GLAPIENTRY _mesa_marshal_{1}({2});'.format(func.return_type, func.name, func.get_parameter_string())) + if func.marshal_no_error: + print('{0} GLAPIENTRY _mesa_marshal_{1}_no_error({2});'.format(func.return_type, func.name, func.get_parameter_string())) def show_usage(): diff --git a/src/mapi/glapi/gen/marshal_XML.py b/src/mapi/glapi/gen/marshal_XML.py index 49314fd14bb..ffaeb1498a9 100644 --- a/src/mapi/glapi/gen/marshal_XML.py +++ b/src/mapi/glapi/gen/marshal_XML.py @@ -116,6 +116,7 @@ class marshal_function(gl_XML.gl_function): self.marshal_call_before = element.get('marshal_call_before') self.marshal_call_after = element.get('marshal_call_after') self.marshal_struct = element.get('marshal_struct') + self.marshal_no_error = gl_XML.is_attr_true(element, 'marshal_no_error') def marshal_flavor(self): """Find out how this function should be marshalled between diff --git a/src/mesa/main/glthread_draw.c b/src/mesa/main/glthread_draw.c index 4e55d8c25ab..99946dd30a2 100644 --- a/src/mesa/main/glthread_draw.c +++ b/src/mesa/main/glthread_draw.c @@ -745,14 +745,20 @@ should_convert_to_begin_end(struct gl_context *ctx, unsigned count, !(vao->NonZeroDivisorMask & vao->BufferEnabled); /* no instanced attribs */ } -static void +static ALWAYS_INLINE void draw_elements(GLuint drawid, GLenum mode, GLsizei count, GLenum type, const GLvoid *indices, GLsizei instance_count, GLint basevertex, GLuint baseinstance, bool index_bounds_valid, GLuint min_index, - GLuint max_index, bool compiled_into_dlist) + GLuint max_index, bool compiled_into_dlist, bool no_error) { GET_CURRENT_CONTEXT(ctx); + /* The main benefit of no_error is that we can discard no-op draws + * immediately. These are plentiful in Viewperf2020/Catia1. + */ + if (no_error && (count <= 0 || instance_count <= 0)) + return; + if (unlikely(compiled_into_dlist && ctx->GLThread.ListMode)) { _mesa_glthread_finish_before(ctx, "DrawElements"); @@ -769,7 +775,7 @@ draw_elements(GLuint drawid, GLenum mode, GLsizei count, GLenum type, return; } - if (unlikely(index_bounds_valid && max_index < min_index)) { + if (unlikely(!no_error && index_bounds_valid && max_index < min_index)) { _mesa_marshal_InternalSetError(GL_INVALID_VALUE); return; } @@ -784,13 +790,14 @@ draw_elements(GLuint drawid, GLenum mode, GLsizei count, GLenum type, * This is also an error path. Zero counts should still call the driver * for possible GL errors. */ - if (count <= 0 || instance_count <= 0 || - !is_index_type_valid(type) || - (!user_buffer_mask && !has_user_indices) || - ctx->Dispatch.Current == ctx->Dispatch.ContextLost || - /* This will just generate GL_INVALID_OPERATION, as it should. */ - ctx->GLThread.inside_begin_end || - ctx->GLThread.ListMode) { + if ((!user_buffer_mask && !has_user_indices) || + (!no_error && + /* zeros are discarded for no_error at the beginning */ + (count <= 0 || instance_count <= 0 || /* GL_INVALID_VALUE / no-op */ + !is_index_type_valid(type) || /* GL_INVALID_VALUE */ + ctx->Dispatch.Current == ctx->Dispatch.ContextLost || /* GL_INVALID_OPERATION */ + ctx->GLThread.inside_begin_end || /* GL_INVALID_OPERATION */ + ctx->GLThread.ListMode))) { /* GL_INVALID_OPERATION */ if (instance_count == 1 && baseinstance == 0 && drawid == 0) { int cmd_size = sizeof(struct marshal_cmd_DrawElementsBaseVertex); struct marshal_cmd_DrawElementsBaseVertex *cmd = @@ -1289,7 +1296,7 @@ lower_draw_elements_indirect(struct gl_context *ctx, GLenum mode, GLenum type, params[i * stride / 4 + 1], params[i * stride / 4 + 3], params[i * stride / 4 + 4], - false, 0, 0, false); + false, 0, 0, false, false); } unmap_draw_indirect_params(ctx); } @@ -1615,7 +1622,16 @@ void GLAPIENTRY _mesa_marshal_DrawElements(GLenum mode, GLsizei count, GLenum type, const GLvoid *indices) { - draw_elements(0, mode, count, type, indices, 1, 0, 0, false, 0, 0, true); + draw_elements(0, mode, count, type, indices, 1, 0, + 0, false, 0, 0, true, false); +} + +void GLAPIENTRY +_mesa_marshal_DrawElements_no_error(GLenum mode, GLsizei count, GLenum type, + const GLvoid *indices) +{ + draw_elements(0, mode, count, type, indices, 1, 0, + 0, false, 0, 0, true, true); } void GLAPIENTRY @@ -1623,21 +1639,51 @@ _mesa_marshal_DrawRangeElements(GLenum mode, GLuint start, GLuint end, GLsizei count, GLenum type, const GLvoid *indices) { - draw_elements(0, mode, count, type, indices, 1, 0, 0, true, start, end, true); + draw_elements(0, mode, count, type, indices, 1, 0, + 0, true, start, end, true, false); +} + +void GLAPIENTRY +_mesa_marshal_DrawRangeElements_no_error(GLenum mode, GLuint start, GLuint end, + GLsizei count, GLenum type, + const GLvoid *indices) +{ + draw_elements(0, mode, count, type, indices, 1, 0, + 0, true, start, end, true, true); } void GLAPIENTRY _mesa_marshal_DrawElementsInstanced(GLenum mode, GLsizei count, GLenum type, const GLvoid *indices, GLsizei instance_count) { - draw_elements(0, mode, count, type, indices, instance_count, 0, 0, false, 0, 0, false); + draw_elements(0, mode, count, type, indices, instance_count, 0, + 0, false, 0, 0, false, false); +} + +void GLAPIENTRY +_mesa_marshal_DrawElementsInstanced_no_error(GLenum mode, GLsizei count, + GLenum type, const GLvoid *indices, + GLsizei instance_count) +{ + draw_elements(0, mode, count, type, indices, instance_count, 0, + 0, false, 0, 0, false, true); } void GLAPIENTRY _mesa_marshal_DrawElementsBaseVertex(GLenum mode, GLsizei count, GLenum type, const GLvoid *indices, GLint basevertex) { - draw_elements(0, mode, count, type, indices, 1, basevertex, 0, false, 0, 0, true); + draw_elements(0, mode, count, type, indices, 1, basevertex, + 0, false, 0, 0, true, false); +} + +void GLAPIENTRY +_mesa_marshal_DrawElementsBaseVertex_no_error(GLenum mode, GLsizei count, + GLenum type, const GLvoid *indices, + GLint basevertex) +{ + draw_elements(0, mode, count, type, indices, 1, basevertex, + 0, false, 0, 0, true, true); } void GLAPIENTRY @@ -1645,7 +1691,17 @@ _mesa_marshal_DrawRangeElementsBaseVertex(GLenum mode, GLuint start, GLuint end, GLsizei count, GLenum type, const GLvoid *indices, GLint basevertex) { - draw_elements(0, mode, count, type, indices, 1, basevertex, 0, true, start, end, true); + draw_elements(0, mode, count, type, indices, 1, basevertex, + 0, true, start, end, true, false); +} + +void GLAPIENTRY +_mesa_marshal_DrawRangeElementsBaseVertex_no_error(GLenum mode, GLuint start, + GLuint end, GLsizei count, GLenum type, + const GLvoid *indices, GLint basevertex) +{ + draw_elements(0, mode, count, type, indices, 1, basevertex, + 0, true, start, end, true, true); } void GLAPIENTRY @@ -1653,7 +1709,17 @@ _mesa_marshal_DrawElementsInstancedBaseVertex(GLenum mode, GLsizei count, GLenum type, const GLvoid *indices, GLsizei instance_count, GLint basevertex) { - draw_elements(0, mode, count, type, indices, instance_count, basevertex, 0, false, 0, 0, false); + draw_elements(0, mode, count, type, indices, instance_count, basevertex, + 0, false, 0, 0, false, false); +} + +void GLAPIENTRY +_mesa_marshal_DrawElementsInstancedBaseVertex_no_error(GLenum mode, GLsizei count, + GLenum type, const GLvoid *indices, + GLsizei instance_count, GLint basevertex) +{ + draw_elements(0, mode, count, type, indices, instance_count, basevertex, + 0, false, 0, 0, false, true); } void GLAPIENTRY @@ -1661,7 +1727,17 @@ _mesa_marshal_DrawElementsInstancedBaseInstance(GLenum mode, GLsizei count, GLenum type, const GLvoid *indices, GLsizei instance_count, GLuint baseinstance) { - draw_elements(0, mode, count, type, indices, instance_count, 0, baseinstance, false, 0, 0, false); + draw_elements(0, mode, count, type, indices, instance_count, 0, + baseinstance, false, 0, 0, false, false); +} + +void GLAPIENTRY +_mesa_marshal_DrawElementsInstancedBaseInstance_no_error(GLenum mode, GLsizei count, + GLenum type, const GLvoid *indices, + GLsizei instance_count, GLuint baseinstance) +{ + draw_elements(0, mode, count, type, indices, instance_count, 0, + baseinstance, false, 0, 0, false, true); } void GLAPIENTRY @@ -1670,7 +1746,18 @@ _mesa_marshal_DrawElementsInstancedBaseVertexBaseInstance(GLenum mode, GLsizei c GLsizei instance_count, GLint basevertex, GLuint baseinstance) { - draw_elements(0, mode, count, type, indices, instance_count, basevertex, baseinstance, false, 0, 0, false); + draw_elements(0, mode, count, type, indices, instance_count, basevertex, + baseinstance, false, 0, 0, false, false); +} + +void GLAPIENTRY +_mesa_marshal_DrawElementsInstancedBaseVertexBaseInstance_no_error(GLenum mode, GLsizei count, + GLenum type, const GLvoid *indices, + GLsizei instance_count, + GLint basevertex, GLuint baseinstance) +{ + draw_elements(0, mode, count, type, indices, instance_count, basevertex, + baseinstance, false, 0, 0, false, true); } void GLAPIENTRY