[Mesa-dev] [PATCH v2 2/2] mesa: Add spec citations for DispatchCompute*

[Jordan Justen]

On 2015-11-02 00:35:36, Jordan Justen wrote:
> Note: The OpenGL 4.3 - 4.5 specification language for DispatchCompute
> appears to have an error regarding the max allowed values. When adding
> the specification citation, we note why the code does not match the
> specification language.
> 
> v2:
>  * Updates based on review from Iago
> 
> Signed-off-by: Jordan Justen <jordan.l.justen at intel.com>
> Cc: Iago Toral Quiroga <itoral at igalia.com>
> Cc: Marta Lofstedt <marta.lofstedt at intel.com>
> ---
>  src/mesa/main/api_validate.c | 34 +++++++++++++++++++++++++++++-----
>  1 file changed, 29 insertions(+), 5 deletions(-)
> 
> diff --git a/src/mesa/main/api_validate.c b/src/mesa/main/api_validate.c
> index a3ee8c0..a490189 100644
> --- a/src/mesa/main/api_validate.c
> +++ b/src/mesa/main/api_validate.c
> @@ -918,6 +918,11 @@ check_valid_to_compute(struct gl_context *ctx, const char *function)
>        return false;
>     }
>  
> +   /* From the OpenGL 4.3 Core Specification, Chapter 19, Compute Shaders:
> +    *
> +    * "An INVALID_OPERATION error is generated if there is no active program
> +    *  for the compute shader stage."
> +    */
>     prog = ctx->_Shader->CurrentProgram[MESA_SHADER_COMPUTE];
>     if (prog == NULL || prog->_LinkedShaders[MESA_SHADER_COMPUTE] == NULL) {
>        _mesa_error(ctx, GL_INVALID_OPERATION,
> @@ -940,6 +945,24 @@ _mesa_validate_DispatchCompute(struct gl_context *ctx,
>        return GL_FALSE;
>  
>     for (i = 0; i < 3; i++) {
> +      /* From the OpenGL 4.3 Core Specification, Chapter 19, Compute Shaders:
> +       *
> +       * "An INVALID_VALUE error is generated if any of num_groups_x,
> +       *  num_groups_y and num_groups_z are greater than or equal to the
> +       *  maximum work group count for the corresponding dimension."
> +       *
> +       * However, the "or equal to" portions appears to be a specification
> +       * bug. In all other areas, the specification appears to indicate that
> +       * the number of workgroups can match the MAX_COMPUTE_WORK_GROUP_COUNT
> +       * value. For example, under DispatchComputeIndirect:
> +       *
> +       * "If any of num_groups_x, num_groups_y or num_groups_z is greater than
> +       *  the value of MAX_COMPUTE_WORK_GROUP_COUNT for the corresponding
> +       *  dimension then the results are undefined."
> +       *
> +       * Additionally, the OpenGLES 3.1 specification does not contain "or
> +       * equal to" as an error condition.

I did get confirmation from the extension spec author that this
appears to be a bug in the OpenGL 4.3 - 4.5 specs, and I filed an
OpenGL spec bug.

-Jordan

> +       */
>        if (num_groups[i] > ctx->Const.MaxComputeWorkGroupCount[i]) {
>           _mesa_error(ctx, GL_INVALID_VALUE,
>                       "glDispatchCompute(num_groups_%c)", 'x' + i);
> @@ -977,6 +1000,12 @@ valid_dispatch_indirect(struct gl_context *ctx,
>        return GL_FALSE;
>     }
>  
> +   /* From the OpenGL 4.3 Core Specification, Chapter 19, Compute Shaders:
> +    *
> +    * "An INVALID_OPERATION error is generated if no buffer is bound to the
> +    *  DRAW_INDIRECT_BUFFER binding, or if the command would source data
> +    *  beyond the end of the buffer object."
> +    */
>     if (!_mesa_is_bufferobj(ctx->DispatchIndirectBuffer)) {
>        _mesa_error(ctx, GL_INVALID_OPERATION,
>                    "%s: no buffer bound to DISPATCH_INDIRECT_BUFFER", name);
> @@ -989,11 +1018,6 @@ valid_dispatch_indirect(struct gl_context *ctx,
>        return GL_FALSE;
>     }
>  
> -   /* From the ARB_compute_shader specification:
> -    *
> -    * "An INVALID_OPERATION error is generated if this command sources data
> -    *  beyond the end of the buffer object [...]"
> -    */
>     if (ctx->DispatchIndirectBuffer->Size < end) {
>        _mesa_error(ctx, GL_INVALID_OPERATION,
>                    "%s(DISPATCH_INDIRECT_BUFFER too small)", name);
> -- 
> 2.6.2
>