[Mesa-dev] [PATCH 4/4] i965/es3.1: Implement glMemoryBarrierByRegion

[Matt Turner]

On Fri, Jul 31, 2015 at 5:15 AM, Marta Lofstedt
<marta.lofstedt at linux.intel.com> wrote:
> From: Marta Lofstedt <marta.lofstedt at intel.com>
>
> Signed-off-by: Marta Lofstedt <marta.lofstedt at intel.com>
> ---
>  src/mesa/drivers/dri/i965/brw_program.c | 34 +++++++++++++++++++++++++++++++++
>  1 file changed, 34 insertions(+)
>
> diff --git a/src/mesa/drivers/dri/i965/brw_program.c b/src/mesa/drivers/dri/i965/brw_program.c
> index 85e271d..332d84e 100644
> --- a/src/mesa/drivers/dri/i965/brw_program.c
> +++ b/src/mesa/drivers/dri/i965/brw_program.c
> @@ -226,6 +226,39 @@ brw_memory_barrier(struct gl_context *ctx, GLbitfield barriers)
>     brw_emit_pipe_control_flush(brw, bits);
>  }
>
> +static void
> +brw_memory_barrier_by_region(struct gl_context *ctx, GLbitfield barriers)
> +{
> +   GLbitfield all_allowed_bits = GL_ATOMIC_COUNTER_BARRIER_BIT |
> +      GL_FRAMEBUFFER_BARRIER_BIT |
> +      GL_SHADER_IMAGE_ACCESS_BARRIER_BIT |
> +      GL_SHADER_STORAGE_BARRIER_BIT |
> +      GL_TEXTURE_FETCH_BARRIER_BIT |
> +      GL_UNIFORM_BARRIER_BIT;

Indent these to match GL_ATOMIC_COUNTER_BARRIER_BIT.

Also, GL_ATOMIC_COUNTER_BARRIER_BIT and GL_SHADER_STORAGE_BARRIER_BIT
are not handled by brw_memory_barrier(). I know the latter is part of
an in-progress feature, but are we missing something for atomic
counters?

> +   /*
> +    * According to OpenGL ES 3.1 spec. April 29, 2015, 7.11.2:
> +    * "When barriers are ALL_BARRIERS_BIT, shader memory access
> +    * will be synchronized realtive to all theese barrier bits,
> +    * but not to other barrier bits specific to MemoryBarrier."

Just copy and paste from the spec to avoid s/is/are/ and typos.

> +    * I.e if bariiers is the special value GL_ALL_BARRIER_BITS,
> +    * then all barriers allowed by glMemoryBarrierByRegion
> +    * should be activated.
> +   */
> +   if (barriers == GL_ALL_BARRIER_BITS)
> +      return brw_memory_barrier(ctx, all_allowed_bits);
> +
> +   /*
> +    * If barriers contain a value that is not allowed
> +    * for glMemoryBarrierByRegion an GL_INVALID_VALUE
> +    * should be generated.
> +   */
> +   if ((all_allowed_bits | barriers) ^ all_allowed_bits)

This might work but I have a hard time thinking about it. The usual pattern is

   (barriers & ~all_allowed_bits) != 0

> +       _mesa_error(ctx, GL_INVALID_VALUE,
> +            "glMemoryBarrierByRegion(unsupported barrier bit");

Indent.

I've attached a patch that cleans up the things I mentioned. Please
squash it in. With that, the patch looks good. I suppose the only
pending question is about the unhandled GL_ATOMIC_COUNTER_BARRIER_BIT
bit.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: p.patch
Type: text/x-patch
Size: 2629 bytes
Desc: not available
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20150731/78d07fe4/attachment.bin>