Jordan Justen <jordan.l.jus...@intel.com> writes: > On 2015-11-05 06:07:02, Francisco Jerez wrote: >> Jordan Justen <jordan.l.jus...@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.jus...@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
signature.asc
Description: PGP signature
_______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev