[Mesa-dev] [PATCH v2] gles/es3.1: Implement glMemoryBarrierByRegion

[Ilia Mirkin]

On Fri, Aug 7, 2015 at 2:18 PM, Matt Turner <mattst88 at gmail.com> wrote:
> On Tue, Aug 4, 2015 at 1:22 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/mapi/glapi/gen/gl_API.xml           |  4 ++++
>>  src/mesa/main/shaderimage.c             | 40 +++++++++++++++++++++++++++++++++
>>  src/mesa/main/shaderimage.h             |  3 +++
>>  src/mesa/main/tests/dispatch_sanity.cpp |  3 +--
>>  4 files changed, 48 insertions(+), 2 deletions(-)
>>
>> diff --git a/src/mapi/glapi/gen/gl_API.xml b/src/mapi/glapi/gen/gl_API.xml
>> index 658efa4..3db4349 100644
>> --- a/src/mapi/glapi/gen/gl_API.xml
>> +++ b/src/mapi/glapi/gen/gl_API.xml
>> @@ -2966,6 +2966,10 @@
>>          <param name="height" type="GLsizei"/>
>>          <glx rop="191"/>
>>      </function>
>> +
>> +    <function name="MemoryBarrierByRegion" es2="3.1">
>> +        <param name="barriers" type="GLbitfield"/>
>> +    </function>
>>  </category>
>>
>>  <category name="1.1">
>> diff --git a/src/mesa/main/shaderimage.c b/src/mesa/main/shaderimage.c
>> index a348cdb..7337f22 100644
>> --- a/src/mesa/main/shaderimage.c
>> +++ b/src/mesa/main/shaderimage.c
>> @@ -653,3 +653,43 @@ _mesa_MemoryBarrier(GLbitfield barriers)
>>     if (ctx->Driver.MemoryBarrier)
>>        ctx->Driver.MemoryBarrier(ctx, barriers);
>>  }
>> +
>> +void GLAPIENTRY
>> +_mesa_MemoryBarrierByRegion(GLbitfield barriers)
>> +{
>> +   GET_CURRENT_CONTEXT(ctx);
>> +
>> +   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;
>> +
>> +   if (ctx->Driver.MemoryBarrier) {
>> +      /* From section 7.11.2 of the OpenGL ES 3.1 specification:
>> +       *
>> +       *    "When barriers is ALL_BARRIER_BITS, shader memory accesses will be
>> +       *     synchronized relative to all these barrier bits, but not to other
>> +       *     barrier bits specific to MemoryBarrier."
>> +       *
>> +       * That is, if barriers is the special value GL_ALL_BARRIER_BITS, then all
>> +       * barriers allowed by glMemoryBarrierByRegion should be activated."
>> +       */
>> +      if (barriers == GL_ALL_BARRIER_BITS)
>> +         return ctx->Driver.MemoryBarrier(ctx, all_allowed_bits);
>> +
>> +      /* From section 7.11.2 of the OpenGL ES 3.1 specification:
>> +       *
>> +       *    "An INVALID_VALUE error is generated if barriers is not the special
>> +       *     value ALL_BARRIER_BITS, and has any bits set other than those
>> +       *     described above."
>> +       */
>> +      if ((barriers & ~all_allowed_bits) != 0) {
>> +         _mesa_error(ctx, GL_INVALID_VALUE,
>> +                     "glMemoryBarrierByRegion(unsupported barrier bit");
>> +      }
>> +
>> +      ctx->Driver.MemoryBarrier(ctx, barriers);
>> +   }
>
> Would probably be nice to put an unreachable("not implemented") as an
> else case for future implementors.
>
> Reviewed-by: Matt Turner <mattst88 at gmail.com>

I wonder if this shouldn't just be

if (!ctx->Driver.MemoryBarrier)
  INVALID_OPERATION

But this is largely hypothetical... I'm not too worried about it.

>
> Thanks!
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev