[Mesa-dev] [PATCH v2 1/3] glsl: Add new barrier functions for compute shaders

[Francisco Jerez]

Jordan Justen <jordan.l.justen at intel.com> writes:

> On 2015-11-05 06:07:02, Francisco Jerez wrote:
>> Jordan Justen <jordan.l.justen at intel.com> writes:
>> 
>> > When these functions are called in GLSL code, we create an intrinsic
>> > function call:
>> >
>> >  * groupMemoryBarrier => __intrinsic_group_memory_barrier
>> >  * memoryBarrierAtomicCounter => __intrinsic_memory_barrier_atomic_counter
>> >  * memoryBarrierBuffer => __intrinsic_memory_barrier_buffer
>> >  * memoryBarrierImage => __intrinsic_memory_barrier_image
>> >  * memoryBarrierShared => __intrinsic_memory_barrier_shared
>> >
>> > v2:
>> >  * Consolidate with memoryBarrier function/intrinsic creation (curro)
>> >
>> > Signed-off-by: Jordan Justen <jordan.l.justen at intel.com>
>> > ---
>> >  src/glsl/builtin_functions.cpp | 58 +++++++++++++++++++++++++++++++++++-------
>> >  1 file changed, 49 insertions(+), 9 deletions(-)
>> >
>> > diff --git a/src/glsl/builtin_functions.cpp b/src/glsl/builtin_functions.cpp
>> > index 509a57b..21ae5ce 100644
>> > --- a/src/glsl/builtin_functions.cpp
>> > +++ b/src/glsl/builtin_functions.cpp
>> > @@ -459,9 +459,15 @@ fp64(const _mesa_glsl_parse_state *state)
>> >  }
>> >  
>> >  static bool
>> > +compute_shader(const _mesa_glsl_parse_state *state)
>> > +{
>> > +   return state->stage == MESA_SHADER_COMPUTE;
>> > +}
>> > +
>> > +static bool
>> >  barrier_supported(const _mesa_glsl_parse_state *state)
>> >  {
>> > -   return state->stage == MESA_SHADER_COMPUTE ||
>> > +   return compute_shader(state) ||
>> >            state->stage == MESA_SHADER_TESS_CTRL;
>> >  }
>> >  
>> > @@ -785,7 +791,9 @@ private:
>> >  
>> >     ir_function_signature *_memory_barrier_intrinsic(
>> >        builtin_available_predicate avail);
>> > -   ir_function_signature *_memory_barrier(
>> > +   void add_memory_barrier_function(
>> > +      const char *function_name,
>> > +      const char *intrinsic_name,
>> >        builtin_available_predicate avail);
>> >  
>> Do we need a separate add_*_function() method for these?  The most
>> consistent with the other builtin builder code would be to have a
>> _memory_barrier() method to construct the ir_function_signature given
>> the name of the intrinsic to call and an availability predicate, and
>> then pass it to the usual add_function() method, kind of like you did in
>> your v1.  Admittedly image load/store/atomic built-ins use a separate
>> add_image_function() for this because they need such a ridiculous amount
>> of overloads -- But that's the exception rather than the rule.
>
> So, add the intrinsic name to lookup to _memory_barrier?
>
> Like this?
>
>    add_function("memoryBarrierAtomicCounter",
>                 _memory_barrier("__intrinsic_memory_barrier_atomic_counter",
>                                 compute_shader),
>                 NULL);
>

Yeah, I think that's the idiom used for all other built-ins generated in
this file, except image load/store/atomic.

> I thought add_memory_barrier_function looked a little better, but
> either is fine with me.
>
*Shrug*, I don't have a strong preference either, but in doubt would go
for the most consistent approach rather than the most convenient. ;)

> -Jordan
>
>> 
>> >     ir_function_signature *_shader_clock_intrinsic(builtin_available_predicate avail,
>> > @@ -963,6 +971,21 @@ builtin_builder::create_intrinsics()
>> >     add_function("__intrinsic_memory_barrier",
>> >                  _memory_barrier_intrinsic(shader_image_load_store),
>> >                  NULL);
>> > +   add_function("__intrinsic_group_memory_barrier",
>> > +                _memory_barrier_intrinsic(compute_shader),
>> > +                NULL);
>> > +   add_function("__intrinsic_memory_barrier_atomic_counter",
>> > +                _memory_barrier_intrinsic(compute_shader),
>> > +                NULL);
>> > +   add_function("__intrinsic_memory_barrier_buffer",
>> > +                _memory_barrier_intrinsic(compute_shader),
>> > +                NULL);
>> > +   add_function("__intrinsic_memory_barrier_image",
>> > +                _memory_barrier_intrinsic(compute_shader),
>> > +                NULL);
>> > +   add_function("__intrinsic_memory_barrier_shared",
>> > +                _memory_barrier_intrinsic(compute_shader),
>> > +                NULL);
>> >  
>> >     add_function("__intrinsic_shader_clock",
>> >                  _shader_clock_intrinsic(shader_clock,
>> > @@ -2753,9 +2776,24 @@ builtin_builder::create_builtins()
>> >  
>> >     add_image_functions(true);
>> >  
>> > -   add_function("memoryBarrier",
>> > -                _memory_barrier(shader_image_load_store),
>> > -                NULL);
>> > +   add_memory_barrier_function("memoryBarrier",
>> > +                               "__intrinsic_memory_barrier",
>> > +                               shader_image_load_store);
>> > +   add_memory_barrier_function("groupMemoryBarrier",
>> > +                               "__intrinsic_group_memory_barrier",
>> > +                               compute_shader);
>> > +   add_memory_barrier_function("memoryBarrierAtomicCounter",
>> > +                               "__intrinsic_memory_barrier_atomic_counter",
>> > +                               compute_shader);
>> > +   add_memory_barrier_function("memoryBarrierBuffer",
>> > +                               "__intrinsic_memory_barrier_buffer",
>> > +                               compute_shader);
>> > +   add_memory_barrier_function("memoryBarrierImage",
>> > +                               "__intrinsic_memory_barrier_image",
>> > +                               compute_shader);
>> > +   add_memory_barrier_function("memoryBarrierShared",
>> > +                               "__intrinsic_memory_barrier_shared",
>> > +                               compute_shader);
>> >  
>> >     add_function("clock2x32ARB",
>> >                  _shader_clock(shader_clock,
>> > @@ -5263,13 +5301,15 @@ builtin_builder::_memory_barrier_intrinsic(builtin_available_predicate avail)
>> >     return sig;
>> >  }
>> >  
>> > -ir_function_signature *
>> > -builtin_builder::_memory_barrier(builtin_available_predicate avail)
>> > +void
>> > +builtin_builder::add_memory_barrier_function(const char *function_name,
>> > +                                             const char *intrinsic_name,
>> > +                                             builtin_available_predicate avail)
>> >  {
>> >     MAKE_SIG(glsl_type::void_type, avail, 0);
>> > -   body.emit(call(shader->symbols->get_function("__intrinsic_memory_barrier"),
>> > +   body.emit(call(shader->symbols->get_function(intrinsic_name),
>> >                    NULL, sig->parameters));
>> > -   return sig;
>> > +   add_function(function_name, sig, NULL);
>> >  }
>> >  
>> >  ir_function_signature *
>> > -- 
>> > 2.6.2
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 212 bytes
Desc: not available
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20151105/c3a8e49e/attachment-0001.sig>