Ilia Mirkin <imir...@alum.mit.edu> writes:
> On Mon, Feb 22, 2016 at 9:40 PM, Francisco Jerez <curroje...@riseup.net>
> wrote:
>> This is basically just the same atomic functions exposed by
>> ARB_shader_image_load_store, with one exception:
>>
>> "highp float imageAtomicExchange(
>> coherent IMAGE_PARAMS,
>> float data);"
>>
>> There's no float atomic exchange overload in the original
>> ARB_shader_image_load_store or GL 4.2, so this seems like new
>> functionality that requires specific back-end support and a separate
>> availability condition in the built-in function generator.
>> ---
>> src/compiler/glsl/builtin_functions.cpp | 23 +++++++++++++++++++----
>> 1 file changed, 19 insertions(+), 4 deletions(-)
>>
>> diff --git a/src/compiler/glsl/builtin_functions.cpp
>> b/src/compiler/glsl/builtin_functions.cpp
>> index f488434..db74672 100644
>> --- a/src/compiler/glsl/builtin_functions.cpp
>> +++ b/src/compiler/glsl/builtin_functions.cpp
>> @@ -448,8 +448,16 @@ shader_image_load_store(const _mesa_glsl_parse_state
>> *state)
>> static bool
>> shader_image_atomic(const _mesa_glsl_parse_state *state)
>> {
>> - return (state->is_version(420, 0) ||
>> - state->ARB_shader_image_load_store_enable);
>> + return (state->is_version(420, 320) ||
>> + state->ARB_shader_image_load_store_enable ||
>> + state->OES_shader_image_atomic_enable);
>> +}
>> +
>> +static bool
>> +shader_image_atomic_exchange_float(const _mesa_glsl_parse_state *state)
>> +{
>> + return (state->is_version(450, 320) ||
>> + state->OES_shader_image_atomic_enable);
>> }
>>
>> static bool
>> @@ -586,6 +594,7 @@ private:
>> IMAGE_FUNCTION_WRITE_ONLY = (1 << 5),
>> IMAGE_FUNCTION_AVAIL_ATOMIC = (1 << 6),
>> IMAGE_FUNCTION_MS_ONLY = (1 << 7),
>> + IMAGE_FUNCTION_AVAIL_ATOMIC_EXCHANGE = (1 << 8)
>> };
>>
>> /**
>> @@ -2981,7 +2990,9 @@ builtin_builder::add_image_functions(bool glsl)
>> add_image_function((glsl ? "imageAtomicExchange" :
>> "__intrinsic_image_atomic_exchange"),
>> "__intrinsic_image_atomic_exchange",
>> - &builtin_builder::_image_prototype, 1, atom_flags);
>> + &builtin_builder::_image_prototype, 1,
>> + (flags | IMAGE_FUNCTION_AVAIL_ATOMIC_EXCHANGE |
>> + IMAGE_FUNCTION_SUPPORTS_FLOAT_DATA_TYPE));
>>
>> add_image_function((glsl ? "imageAtomicCompSwap" :
>> "__intrinsic_image_atomic_comp_swap"),
>> @@ -5250,7 +5261,11 @@ builtin_builder::_image_prototype(const glsl_type
>> *image_type,
>> glsl_type::ivec(image_type->coordinate_components()), "coord");
>>
>> const builtin_available_predicate avail =
>> - (flags & IMAGE_FUNCTION_AVAIL_ATOMIC ? shader_image_atomic :
>> + (((flags & IMAGE_FUNCTION_AVAIL_ATOMIC_EXCHANGE) &&
>> + image_type->sampled_type == GLSL_TYPE_FLOAT) ?
>> + shader_image_atomic_exchange_float :
>> + flags & (IMAGE_FUNCTION_AVAIL_ATOMIC_EXCHANGE |
>> + IMAGE_FUNCTION_AVAIL_ATOMIC) ? shader_image_atomic :
>> shader_image_load_store);
>
> Wow, this is a monster expression. I know how much you like "pure"
> const values, but perhaps this might be more readable as a bunch of
> if's? I think you can actually do
>
> const foo; // no init
> if () foo = a
> else foo = b
>
> type of stuff. Your call though.
>
Yeah, you're right the expression was rather hard to read, but I was
thinking this is turning into kind of a monster function anyway? How
about I move the calculation of the availability predicate into a
separate function? v2 coming up in a minute.> For some reason in my version I had decided to add a separate > add_image_float_functions, but in hindsight that may have been > misguided, as this seems to be sufficient. > >> ir_function_signature *sig = new_sig(ret_type, avail, 2, image, coord); >> >> -- >> 2.7.0 >>
signature.asc
Description: PGP signature
_______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
