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

Lofstedt, Marta marta.lofstedt at intel.com
Fri Aug 14 03:27:04 PDT 2015


> -----Original Message-----
> From: mesa-dev [mailto:mesa-dev-bounces at lists.freedesktop.org] On
> Behalf Of Tapani Pälli
> Sent: Friday, August 14, 2015 7:31 AM
> To: Marta Lofstedt; mesa-dev at lists.freedesktop.org
> Subject: Re: [Mesa-dev] [PATCH v2] gles/es3.1: Implement
> glMemoryBarrierByRegion
> 
> 
> 
> On 08/14/2015 08:21 AM, Tapani Pälli wrote:
> >
> >
> > On 08/07/2015 12:56 PM, Tapani Pälli wrote:
> >> Super, I've verified that this makes
> >> ES31-CTS.shader_image_load_store.basic-api-barrier-byRegion pass with
> >> one of Curro's branches.
> >>
> >> Reviewed-by: Tapani Pälli <tapani.palli at intel.com>
> >
> > Urgh should've tested this before but now I noticed that 'make check'
> > does not pass with this.
> >
> > This function is part of OpenGL 4.5 so we need to move it to GL4x.xml
> > and expose it in gl_core_functions_possible in dispatch_sanity.cpp
> > (with version 45). However, 'make check' still fails with these
> > changes, I'm trying to figure out why. The failing test is GL30 where
> > function ends up (no matter what GL version specified in entry).
> 
> I found that make check passes if I add 'desktop=false' to the xml entry
> making this function ES only. I guess we should go and do that to get forward.
> 
Hmm, desktop="false", certainly fix the old patch. But, since
glMemoryBarrierByRegion is now also in GL 4.5, that does not make sense. 
I have tested moving to GL4.xml and adding to dispatch_sanity with 45,
but then as you wrote, it is exposed under GL 3.0. 

I am really anxious to get this in, since the GLES 3.1 CTS segfaults without it.

I could send up a V3 with the desktop=false solution, but I don't think that 
is correct, I need to figure out how to have under GL 4.5 and GLES 3.1 only.

Anyways, thanks for your findings Tapani.
> 
> >
> >> On 08/04/2015 11:22 AM, Marta Lofstedt 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);
> >>> +   }
> >>> +}
> >>> diff --git a/src/mesa/main/shaderimage.h
> >>> b/src/mesa/main/shaderimage.h index 33d8a1e..d08ece8 100644
> >>> --- a/src/mesa/main/shaderimage.h
> >>> +++ b/src/mesa/main/shaderimage.h
> >>> @@ -68,6 +68,9 @@ _mesa_BindImageTextures(GLuint first, GLsizei
> >>> count, const GLuint *textures);
> >>>   void GLAPIENTRY
> >>>   _mesa_MemoryBarrier(GLbitfield barriers);
> >>>
> >>> +void GLAPIENTRY
> >>> +_mesa_MemoryBarrierByRegion(GLbitfield barriers);
> >>> +
> >>>   #ifdef __cplusplus
> >>>   }
> >>>   #endif
> >>> diff --git a/src/mesa/main/tests/dispatch_sanity.cpp
> >>> b/src/mesa/main/tests/dispatch_sanity.cpp
> >>> index af89d2c..14c9eda 100644
> >>> --- a/src/mesa/main/tests/dispatch_sanity.cpp
> >>> +++ b/src/mesa/main/tests/dispatch_sanity.cpp
> >>> @@ -2461,8 +2461,7 @@ const struct function
> >>> gles31_functions_possible[] = {
> >>>      { "glGetBooleani_v", 31, -1 },
> >>>      { "glMemoryBarrier", 31, -1 },
> >>>
> >>> -   // FINISHME: This function has not been implemented yet.
> >>> -   // { "glMemoryBarrierByRegion", 31, -1 },
> >>> +   { "glMemoryBarrierByRegion", 31, -1 },
> >>>
> >>>      { "glTexStorage2DMultisample", 31, -1 },
> >>>      { "glGetMultisamplefv", 31, -1 },
> >>>
> >> _______________________________________________
> >> mesa-dev mailing list
> >> mesa-dev at lists.freedesktop.org
> >> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
> > _______________________________________________
> > mesa-dev mailing list
> > mesa-dev at lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/mesa-dev
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev


More information about the mesa-dev mailing list