[Mesa-dev] [PATCH 12/20] mesa: implement buffer/texture barriers for semaphore signal/wait v2

[Emil Velikov]

On 31 January 2018 at 15:20, Jon Turney <jon.turney at dronecode.org.uk> wrote:
> On 23/01/2018 18:05, Andres Rodriguez wrote:
>>
>> Make sure memory is accessible to the external client, for the specified
>> memory object, before the signal/after the wait.
>>
>> v2: fixed flush order with respect to wait/signal emission
>>
>> Signed-off-by: Andres Rodriguez <andresx7 at gmail.com>
>> ---
>>   src/mesa/main/dd.h                              | 14 ++++++-
>>   src/mesa/main/externalobjects.c                 | 38 +++++++++++++++---
>>   src/mesa/state_tracker/st_cb_semaphoreobjects.c | 53
>> +++++++++++++++++++++++--
>>   3 files changed, 95 insertions(+), 10 deletions(-)
>
> [...]
>
>> diff --git a/src/mesa/main/externalobjects.c
>> b/src/mesa/main/externalobjects.c
>> index 4fb3ca07a9..c070d7a28d 100644
>> --- a/src/mesa/main/externalobjects.c
>> +++ b/src/mesa/main/externalobjects.c
>> @@ -23,6 +23,7 @@
>>     #include "macros.h"
>>   #include "mtypes.h"
>> +#include "bufferobj.h"
>>   #include "context.h"
>>   #include "externalobjects.h"
>>   #include "teximage.h"
>> @@ -716,7 +717,8 @@ _mesa_WaitSemaphoreEXT(GLuint semaphore,
>>   {
>>      GET_CURRENT_CONTEXT(ctx);
>>      struct gl_semaphore_object *semObj;
>> -
>> +   struct gl_buffer_object **bufObjs;
>> +   struct gl_texture_object **texObjs;
>>        if (!ctx->Extensions.EXT_semaphore) {
>>         _mesa_error(ctx, GL_INVALID_OPERATION,
>> "glWaitSemaphoreEXT(unsupported)");
>> @@ -732,8 +734,20 @@ _mesa_WaitSemaphoreEXT(GLuint semaphore,
>>      FLUSH_VERTICES( ctx, 0 );
>>      FLUSH_CURRENT( ctx, 0 );
>>   -   /* TODO: memory barriers and layout transitions */
>> -   ctx->Driver.ServerWaitSemaphoreObject(ctx, semObj);
>> +   bufObjs = alloca(sizeof(struct gl_buffer_object **) *
>> numBufferBarriers);
>> +   for (unsigned i = 0; i < numBufferBarriers; i++) {
>> +      bufObjs[i] = _mesa_lookup_bufferobj(ctx, buffers[i]);
>> +   }
>> +
>> +   texObjs = alloca(sizeof(struct gl_texture_object **) *
>> numTextureBarriers);
>> +   for (unsigned i = 0; i < numTextureBarriers; i++) {
>> +      texObjs[i] = _mesa_lookup_texture(ctx, textures[i]);
>> +   }
>> +
>> +   ctx->Driver.ServerWaitSemaphoreObject(ctx, semObj,
>> +                                         numBufferBarriers, bufObjs,
>> +                                         numTextureBarriers, texObjs,
>> +                                         srcLayouts);
>>   }
>>     void GLAPIENTRY
>> @@ -746,6 +760,8 @@ _mesa_SignalSemaphoreEXT(GLuint semaphore,
>>   {
>>      GET_CURRENT_CONTEXT(ctx);
>>      struct gl_semaphore_object *semObj;
>> +   struct gl_buffer_object **bufObjs;
>> +   struct gl_texture_object **texObjs;
>>        if (!ctx->Extensions.EXT_semaphore) {
>>         _mesa_error(ctx, GL_INVALID_OPERATION,
>> "glSignalSemaphoreEXT(unsupported)");
>> @@ -761,8 +777,20 @@ _mesa_SignalSemaphoreEXT(GLuint semaphore,
>>      FLUSH_VERTICES( ctx, 0 );
>>      FLUSH_CURRENT( ctx, 0 );
>>   -   /* TODO: memory barriers and layout transitions */
>> -   ctx->Driver.ServerSignalSemaphoreObject(ctx, semObj);
>> +   bufObjs = alloca(sizeof(struct gl_buffer_object **) *
>> numBufferBarriers);
>> +   for (unsigned i = 0; i < numBufferBarriers; i++) {
>> +      bufObjs[i] = _mesa_lookup_bufferobj(ctx, buffers[i]);
>> +   }
>> +
>> +   texObjs = alloca(sizeof(struct gl_texture_object **) *
>> numTextureBarriers);
>> +   for (unsigned i = 0; i < numTextureBarriers; i++) {
>> +      texObjs[i] = _mesa_lookup_texture(ctx, textures[i]);
>> +   }
>> +
>> +   ctx->Driver.ServerSignalSemaphoreObject(ctx, semObj,
>> +                                           numBufferBarriers, bufObjs,
>> +                                           numTextureBarriers, texObjs,
>> +                                           dstLayouts);
>>   }
>
> [...]
>
> This adds a use of alloca(), without a corresponding #include.
>
> Patch attached.
>
> See also:
> https://lists.freedesktop.org/archives/mesa-dev/2017-December/180073.html
> https://lists.freedesktop.org/archives/mesa-dev/2016-July/122346.html
>
We have only a few instances in alloca in-tree. The num* variables are
user provided, so it's very likely to wreck chaos.
I'd stick with malloc, esp. considering the different alloca
implementations and header madness.

-Emil