[Mesa-dev] [PATCH 02/11] mesa/main: add support for ARB_compute_variable_groups_size
Ian Romanick
idr at freedesktop.org
Thu Sep 8 20:58:35 UTC 2016
On 09/08/2016 01:31 PM, Samuel Pitoiset wrote:
> Signed-off-by: Samuel Pitoiset <samuel.pitoiset at gmail.com>
> ---
> src/mesa/main/api_validate.c | 94 ++++++++++++++++++++++++++++++++++++++++
> src/mesa/main/api_validate.h | 4 ++
> src/mesa/main/compute.c | 17 ++++++++
> src/mesa/main/context.c | 6 +++
> src/mesa/main/dd.h | 9 ++++
> src/mesa/main/extensions_table.h | 1 +
> src/mesa/main/get.c | 12 +++++
> src/mesa/main/get_hash_params.py | 3 ++
> src/mesa/main/mtypes.h | 23 +++++++++-
> src/mesa/main/shaderapi.c | 1 +
> src/mesa/main/shaderobj.c | 2 +
> 11 files changed, 171 insertions(+), 1 deletion(-)
>
> diff --git a/src/mesa/main/api_validate.c b/src/mesa/main/api_validate.c
> index b35751e..9379015 100644
> --- a/src/mesa/main/api_validate.c
> +++ b/src/mesa/main/api_validate.c
> @@ -1096,6 +1096,7 @@ GLboolean
> _mesa_validate_DispatchCompute(struct gl_context *ctx,
> const GLuint *num_groups)
> {
> + struct gl_shader_program *prog;
> int i;
> FLUSH_CURRENT(ctx, 0);
>
> @@ -1128,6 +1129,86 @@ _mesa_validate_DispatchCompute(struct gl_context *ctx,
> }
> }
>
> + /* From the ARB_compute_variable_group_size specification:
> + *
> + * "An INVALID_OPERATION error is generated by DispatchCompute if the active
> + * program for the compute shader stage has a variable work group size."
> + */
There has been a lot of debate about formatting spec quotations. The
one thing where I think everyone agrees is formatting the first like.
Please use
The ARB_compute_variable_group_size spec says:
That makes it much easier to grep the code for spec quotations.
This comment applies to the spec quotations below too.
> + prog = ctx->_Shader->CurrentProgram[MESA_SHADER_COMPUTE];
> + if (prog->Comp.LocalSizeVariable) {
> + _mesa_error(ctx, GL_INVALID_OPERATION,
> + "glDispatchCompute(variable work group size forbidden)");
> + return GL_FALSE;
> + }
> +
> + return GL_TRUE;
> +}
> +
> +GLboolean
> +_mesa_validate_DispatchComputeGroupSizeARB(struct gl_context *ctx,
> + const GLuint *num_groups,
> + const GLuint *group_size)
> +{
> + struct gl_shader_program *prog;
> + GLuint64 total_invocations = 1;
> + int i;
> +
> + FLUSH_CURRENT(ctx, 0);
> +
> + if (!check_valid_to_compute(ctx, "glDispatchComputeGroupSizeARB"))
> + return GL_FALSE;
> +
> + for (i = 0; i < 3; i++) {
> + /* From the ARB_compute_variable_group_size specification:
> + *
> + * "An INVALID_VALUE error is generated by DispatchComputeGroupSizeARB if
> + * any of <group_size_x>, <group_size_y>, or <group_size_z> is less than
> + * or equal to zero or greater than the maximum local work group size for
> + * compute shaders with variable group size
> + * (MAX_COMPUTE_VARIABLE_GROUP_SIZE_ARB) in the corresponding dimension."
> + *
> + * However, the "less than" is a spec bug because they are declared as
> + * unsigned integers.
> + */
> + if (group_size[i] == 0 ||
> + group_size[i] > ctx->Const.MaxComputeVariableGroupSize[i]) {
> + _mesa_error(ctx, GL_INVALID_VALUE,
> + "glDispatchComputeGroupSizeARB(group_size_%c)", 'x' + i);
> + return GL_FALSE;
> + }
> +
> + /* From the ARB_compute_variable_group_size specification:
> + *
> + * "An INVALID_VALUE error is generated by DispatchComputeGroupSizeARB if
> + * the product of <group_size_x>, <group_size_y>, and <group_size_z>
> + * exceeds the implementation-dependent maximum local work group
> + * invocation count for compute shaders with variable group size
> + * (MAX_COMPUTE_VARIABLE_GROUP_INVOCATIONS_ARB)."
> + */
> + total_invocations *= group_size[i];
> + if (total_invocations > ctx->Const.MaxComputeVariableGroupInvocations) {
> + _mesa_error(ctx, GL_INVALID_VALUE,
> + "glDispatchComputeGroupSizeARB(product of local_sizes "
> + "exceeds MAX_COMPUTE_VARIABLE_GROUP_INVOCATIONS_ARB (%d))",
> + ctx->Const.MaxComputeVariableGroupInvocations);
> + return GL_FALSE;
> + }
This check should happen after the loop, and you should also log the
value of total_invocations. Something like:
_mesa_error(ctx, GL_INVALID_VALUE,
"glDispatchComputeGroupSizeARB(product of local_sizes "
"exceeds MAX_COMPUTE_VARIABLE_GROUP_INVOCATIONS_ARB (%d < %d))",
total_invocations,
ctx->Const.MaxComputeVariableGroupInvocations);
> +
> + /* From the ARB_compute_variable_group_size specification:
> + *
> + * "An INVALID_OPERATION error is generated by
> + * DispatchComputeGroupSizeARB if the active program for the compute
> + * shader stage has a fixed work group size."
> + */
> + prog = ctx->_Shader->CurrentProgram[MESA_SHADER_COMPUTE];
> + if (prog->Comp.LocalSize[i] != 0) {
Does LocalSize[i] == 0 imply !LocalSizeVariable? Is LocalSizeVariable
redundant?
> + _mesa_error(ctx, GL_INVALID_OPERATION,
> + "glDispatchComputeGroupSizeARB(fixed work group size "
> + "forbidden)");
> + return GL_FALSE;
> + }
> + }
> +
> return GL_TRUE;
> }
>
> @@ -1137,6 +1218,7 @@ valid_dispatch_indirect(struct gl_context *ctx,
> GLsizei size, const char *name)
> {
> const uint64_t end = (uint64_t) indirect + size;
> + struct gl_shader_program *prog;
>
> if (!check_valid_to_compute(ctx, name))
> return GL_FALSE;
> @@ -1182,6 +1264,18 @@ valid_dispatch_indirect(struct gl_context *ctx,
> return GL_FALSE;
> }
>
> + /* From the ARB_compute_variable_group_size specification:
> + *
> + * "An INVALID_OPERATION error is generated if the active program for the
> + * compute shader stage has a variable work group size."
> + */
> + prog = ctx->_Shader->CurrentProgram[MESA_SHADER_COMPUTE];
> + if (prog->Comp.LocalSizeVariable) {
> + _mesa_error(ctx, GL_INVALID_OPERATION,
> + "%s(variable work group size forbidden)", name);
> + return GL_FALSE;
> + }
> +
> return GL_TRUE;
> }
>
> diff --git a/src/mesa/main/api_validate.h b/src/mesa/main/api_validate.h
> index 5b321e3..16bcbf6 100644
> --- a/src/mesa/main/api_validate.h
> +++ b/src/mesa/main/api_validate.h
> @@ -129,5 +129,9 @@ extern GLboolean
> _mesa_validate_DispatchComputeIndirect(struct gl_context *ctx,
> GLintptr indirect);
>
> +extern GLboolean
> +_mesa_validate_DispatchComputeGroupSizeARB(struct gl_context *ctx,
> + const GLuint *num_groups,
> + const GLuint *group_size);
>
> #endif
> diff --git a/src/mesa/main/compute.c b/src/mesa/main/compute.c
> index b052bae..bb62539 100644
> --- a/src/mesa/main/compute.c
> +++ b/src/mesa/main/compute.c
> @@ -66,5 +66,22 @@ _mesa_DispatchComputeGroupSizeARB(GLuint num_groups_x, GLuint num_groups_y,
> GLuint num_groups_z, GLuint group_size_x,
> GLuint group_size_y, GLuint group_size_z)
> {
> + GET_CURRENT_CONTEXT(ctx);
> + const GLuint num_groups[3] = { num_groups_x, num_groups_y, num_groups_z };
> + const GLuint group_size[3] = { group_size_x, group_size_y, group_size_z };
> +
> + if (MESA_VERBOSE & VERBOSE_API)
> + _mesa_debug(ctx,
> + "glDispatchComputeGroupSizeARB(%d, %d, %d, %d, %d, %d)\n",
> + num_groups_x, num_groups_y, num_groups_z,
> + group_size_x, group_size_y, group_size_z);
> +
> + if (!_mesa_validate_DispatchComputeGroupSizeARB(ctx, num_groups,
> + group_size))
> + return;
> +
> + if (num_groups_x == 0u || num_groups_y == 0u || num_groups_z == 0u)
> + return;
>
> + ctx->Driver.DispatchComputeGroupSize(ctx, num_groups, group_size);
> }
> diff --git a/src/mesa/main/context.c b/src/mesa/main/context.c
> index f550b0c..11a8ad0 100644
> --- a/src/mesa/main/context.c
> +++ b/src/mesa/main/context.c
> @@ -724,6 +724,12 @@ _mesa_init_constants(struct gl_constants *consts, gl_api api)
> consts->MaxTessPatchComponents = MAX_TESS_PATCH_COMPONENTS;
> consts->MaxTessControlTotalOutputComponents = MAX_TESS_CONTROL_TOTAL_OUTPUT_COMPONENTS;
> consts->PrimitiveRestartForPatches = false;
> +
> + /** GL_ARB_compute_variable_group_size */
> + consts->MaxComputeVariableGroupSize[0] = 512;
> + consts->MaxComputeVariableGroupSize[1] = 512;
> + consts->MaxComputeVariableGroupSize[2] = 64;
> + consts->MaxComputeVariableGroupInvocations = 512;
> }
>
>
> diff --git a/src/mesa/main/dd.h b/src/mesa/main/dd.h
> index 257dc10..7f53271 100644
> --- a/src/mesa/main/dd.h
> +++ b/src/mesa/main/dd.h
> @@ -978,6 +978,15 @@ struct dd_function_table {
> /*@}*/
>
> /**
> + * \name GL_ARB_compute_variable_group_size interface
> + */
> + /*@{*/
> + void (*DispatchComputeGroupSize)(struct gl_context *ctx,
> + const GLuint *num_groups,
> + const GLuint *group_size);
> + /*@}*/
> +
> + /**
> * Query information about memory. Device memory is e.g. VRAM. Staging
> * memory is e.g. GART. All sizes are in kilobytes.
> */
> diff --git a/src/mesa/main/extensions_table.h b/src/mesa/main/extensions_table.h
> index 75cdcb8..b758b81 100644
> --- a/src/mesa/main/extensions_table.h
> +++ b/src/mesa/main/extensions_table.h
> @@ -40,6 +40,7 @@ EXT(ARB_clip_control , ARB_clip_control
> EXT(ARB_color_buffer_float , ARB_color_buffer_float , GLL, GLC, x , x , 2004)
> EXT(ARB_compressed_texture_pixel_storage , dummy_true , GLL, GLC, x , x , 2011)
> EXT(ARB_compute_shader , ARB_compute_shader , GLL, GLC, x , x , 2012)
> +EXT(ARB_compute_variable_group_size , ARB_compute_variable_group_size , GLL, GLC, x , x , 2013)
> EXT(ARB_conditional_render_inverted , ARB_conditional_render_inverted , GLL, GLC, x , x , 2014)
> EXT(ARB_conservative_depth , ARB_conservative_depth , GLL, GLC, x , x , 2011)
> EXT(ARB_copy_buffer , dummy_true , GLL, GLC, x , x , 2008)
> diff --git a/src/mesa/main/get.c b/src/mesa/main/get.c
> index 810ccb9..9423bb4 100644
> --- a/src/mesa/main/get.c
> +++ b/src/mesa/main/get.c
> @@ -470,6 +470,7 @@ EXTRA_EXT(ARB_cull_distance);
> EXTRA_EXT(EXT_window_rectangles);
> EXTRA_EXT(KHR_blend_equation_advanced_coherent);
> EXTRA_EXT(OES_primitive_bounding_box);
> +EXTRA_EXT(ARB_compute_variable_group_size);
>
> static const int
> extra_ARB_color_buffer_float_or_glcore[] = {
> @@ -2266,6 +2267,17 @@ find_value_indexed(const char *func, GLenum pname, GLuint index, union value *v)
> goto invalid_value;
> v->value_int = ctx->Const.MaxComputeWorkGroupSize[index];
> return TYPE_INT;
> +
> + /* ARB_compute_variable_group_size */
> + case GL_MAX_COMPUTE_VARIABLE_GROUP_SIZE_ARB:
> + if (!_mesa_has_compute_shaders(ctx))
> + goto invalid_enum;
> + if (!ctx->Extensions.ARB_compute_variable_group_size)
> + goto invalid_enum;
> + if (index >= 3)
> + goto invalid_value;
> + v->value_int = ctx->Const.MaxComputeVariableGroupSize[index];
> + return TYPE_INT;
> }
>
> invalid_enum:
> diff --git a/src/mesa/main/get_hash_params.py b/src/mesa/main/get_hash_params.py
> index a206c85..dabb91c 100644
> --- a/src/mesa/main/get_hash_params.py
> +++ b/src/mesa/main/get_hash_params.py
> @@ -929,6 +929,9 @@ descriptor=[
> # GL_ARB_cull_distance
> [ "MAX_CULL_DISTANCES", "CONTEXT_INT(Const.MaxClipPlanes), extra_ARB_cull_distance" ],
> [ "MAX_COMBINED_CLIP_AND_CULL_DISTANCES", "CONTEXT_INT(Const.MaxClipPlanes), extra_ARB_cull_distance" ],
> +
> +# GL_ARB_compute_variable_group_size
> + [ "MAX_COMPUTE_VARIABLE_GROUP_INVOCATIONS_ARB", "CONTEXT_INT(Const.MaxComputeVariableGroupInvocations), extra_ARB_compute_variable_group_size" ],
> ]},
>
> # Enums restricted to OpenGL Core profile
> diff --git a/src/mesa/main/mtypes.h b/src/mesa/main/mtypes.h
> index df93446..e3fdf9e 100644
> --- a/src/mesa/main/mtypes.h
> +++ b/src/mesa/main/mtypes.h
> @@ -2077,6 +2077,11 @@ struct gl_compute_program
> * Size of shared variables accessed by the compute shader.
> */
> unsigned SharedSize;
> +
> + /**
> + * Whether a variable work group size has been specified.
> + */
> + bool LocalSizeVariable;
> };
>
>
> @@ -2339,7 +2344,8 @@ struct gl_shader_info
> GLbitfield BlendSupport;
>
> /**
> - * Compute shader state from ARB_compute_shader layout qualifiers.
> + * Compute shader state from ARB_compute_shader and
> + * ARB_compute_variable_group_size layout qualifiers.
> */
> struct {
> /**
> @@ -2347,6 +2353,12 @@ struct gl_shader_info
> * it's not set in this shader.
> */
> unsigned LocalSize[3];
> +
> + /**
> + * Whether a variable work group size has been specified as defined by
> + * ARB_compute_variable_group_size.
> + */
> + bool LocalSizeVariable;
> } Comp;
> };
>
> @@ -2811,6 +2823,10 @@ struct gl_shader_program
> * Size of shared variables accessed by the compute shader.
> */
> unsigned SharedSize;
Blank line here.
> + /**
> + * Whether a variable work group size has been specified.
> + */
> + bool LocalSizeVariable;
> } Comp;
>
> /* post-link info: */
> @@ -3768,6 +3784,10 @@ struct gl_constants
> GLuint MaxComputeWorkGroupInvocations;
> GLuint MaxComputeSharedMemorySize;
>
> + /** GL_ARB_compute_variable_group_size */
> + GLuint MaxComputeVariableGroupSize[3]; /* Array of x, y, z dimensions */
> + GLuint MaxComputeVariableGroupInvocations;
> +
> /** GL_ARB_gpu_shader5 */
> GLfloat MinFragmentInterpolationOffset;
> GLfloat MaxFragmentInterpolationOffset;
> @@ -3819,6 +3839,7 @@ struct gl_extensions
> GLboolean ARB_clip_control;
> GLboolean ARB_color_buffer_float;
> GLboolean ARB_compute_shader;
> + GLboolean ARB_compute_variable_group_size;
> GLboolean ARB_conditional_render_inverted;
> GLboolean ARB_conservative_depth;
> GLboolean ARB_copy_image;
> diff --git a/src/mesa/main/shaderapi.c b/src/mesa/main/shaderapi.c
> index 4ebc39f..5221ef1 100644
> --- a/src/mesa/main/shaderapi.c
> +++ b/src/mesa/main/shaderapi.c
> @@ -2207,6 +2207,7 @@ _mesa_copy_linked_program_data(gl_shader_stage type,
> for (i = 0; i < 3; i++)
> dst_cp->LocalSize[i] = src->Comp.LocalSize[i];
> dst_cp->SharedSize = src->Comp.SharedSize;
> + dst_cp->LocalSizeVariable = src->Comp.LocalSizeVariable;
> break;
> }
> default:
> diff --git a/src/mesa/main/shaderobj.c b/src/mesa/main/shaderobj.c
> index 0075a6d..451e719 100644
> --- a/src/mesa/main/shaderobj.c
> +++ b/src/mesa/main/shaderobj.c
> @@ -262,6 +262,8 @@ init_shader_program(struct gl_shader_program *prog)
> prog->Geom.UsesEndPrimitive = false;
> prog->Geom.UsesStreams = false;
>
> + prog->Comp.LocalSizeVariable = false;
> +
> prog->TransformFeedback.BufferMode = GL_INTERLEAVED_ATTRIBS;
>
> exec_list_make_empty(&prog->EmptyUniformLocations);
>
More information about the mesa-dev
mailing list