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

Dave Airlie airlied at gmail.com
Fri Aug 14 03:42:38 PDT 2015


On 14 August 2015 at 20:27, Lofstedt, Marta <marta.lofstedt at intel.com> wrote:
>> -----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.

you might need to add it to apiexec.py if you are making it core only,

also apiexec.py doesn't require the gl prefix.

the problems you are seeing with make check sound the same as the ones
I had with subroutines.

Dave.

>
> 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
> _______________________________________________
> 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