[Mesa-dev] [PATCH v4] glsl: update the extensions that are enabled for 460
Samuel Pitoiset
samuel.pitoiset at gmail.com
Mon Aug 7 09:49:54 UTC 2017
On 08/07/2017 09:25 AM, Nicolai Hähnle wrote:
> On 04.08.2017 17:46, Ilia Mirkin wrote:
>> On Fri, Aug 4, 2017 at 10:49 AM, Samuel Pitoiset
>> <samuel.pitoiset at gmail.com> wrote:
>>>
>>>
>>> On 08/04/2017 04:27 PM, Ilia Mirkin wrote:
>>>>
>>>>
>>>>
>>>> On Aug 4, 2017 02:02, "Samuel Pitoiset" <samuel.pitoiset at gmail.com
>>>> <mailto:samuel.pitoiset at gmail.com>> wrote:
>>>>
>>>>
>>>>
>>>> On 08/03/2017 07:36 PM, Ilia Mirkin wrote:
>>>>
>>>> On Thu, Aug 3, 2017 at 5:24 AM, Samuel Pitoiset
>>>> <samuel.pitoiset at gmail.com <mailto:samuel.pitoiset at gmail.com>>
>>>> wrote:
>>>>
>>>> Other ones are either unsupported or don't have any helper
>>>> function checks.
>>>>
>>>> v4: - drop ARB suffix for
>>>> shader_group_vote/arb_shader_atomic_counter_ops
>>>> v3: - always add gl_BaseVertex & co when 460 is enabled
>>>> v2: - fix ARB_shader_draw_parameters system value names
>>>>
>>>> Signed-off-by: Samuel Pitoiset <samuel.pitoiset at gmail.com
>>>> <mailto:samuel.pitoiset at gmail.com>>
>>>>
>>>> ---
>>>> src/compiler/glsl/builtin_functions.cpp | 81
>>>> +++++++++++++++++++++++++++++----
>>>> src/compiler/glsl/builtin_variables.cpp | 5 ++
>>>> 2 files changed, 78 insertions(+), 8 deletions(-)
>>>>
>>>> diff --git a/src/compiler/glsl/builtin_functions.cpp
>>>> b/src/compiler/glsl/builtin_functions.cpp
>>>> index 84833bdd7d..bbb60b4e64 100644
>>>> --- a/src/compiler/glsl/builtin_functions.cpp
>>>> +++ b/src/compiler/glsl/builtin_functions.cpp
>>>> @@ -151,6 +151,12 @@ v130_desktop(const
>>>> _mesa_glsl_parse_state *state)
>>>> }
>>>>
>>>> static bool
>>>> +v460_desktop(const _mesa_glsl_parse_state *state)
>>>> +{
>>>> + return state->is_version(460, 0);
>>>> +}
>>>> +
>>>> +static bool
>>>> v130_fs_only(const _mesa_glsl_parse_state *state)
>>>> {
>>>> return state->is_version(130, 300) &&
>>>> @@ -483,7 +489,7 @@ shader_atomic_counters(const
>>>> _mesa_glsl_parse_state *state)
>>>> static bool
>>>> shader_atomic_counter_ops(const _mesa_glsl_parse_state
>>>> *state)
>>>> {
>>>> - return state->ARB_shader_atomic_counter_ops_enable;
>>>> + return v460_desktop(state) ||
>>>> state->ARB_shader_atomic_counter_ops_enable;
>>>> }
>>>>
>>>> static bool
>>>> @@ -606,7 +612,7 @@ barrier_supported(const
>>>> _mesa_glsl_parse_state *state)
>>>> static bool
>>>> vote(const _mesa_glsl_parse_state *state)
>>>> {
>>>> - return state->ARB_shader_group_vote_enable;
>>>> + return v460_desktop(state) ||
>>>> state->ARB_shader_group_vote_enable;
>>>> }
>>>>
>>>> static bool
>>>> @@ -962,7 +968,8 @@ private:
>>>>
>>>> ir_function_signature
>>>> *_vote_intrinsic(builtin_available_predicate avail,
>>>> enum
>>>> ir_intrinsic_id id);
>>>> - ir_function_signature *_vote(const char
>>>> *intrinsic_name);
>>>> + ir_function_signature *_vote(const char
>>>> *intrinsic_name,
>>>> +
>>>> builtin_available_predicate
>>>> avail);
>>>>
>>>> #undef B0
>>>> #undef B1
>>>> @@ -3031,6 +3038,43 @@ builtin_builder::create_builtins()
>>>>
>>>> shader_atomic_counter_ops),
>>>> NULL);
>>>>
>>>> + add_function("atomicCounterAdd",
>>>> +
>>>> _atomic_counter_op1("__intrinsic_atomic_add",
>>>> + v460_desktop),
>>>> + NULL);
>>>> + add_function("atomicCounterSubtract",
>>>> +
>>>> _atomic_counter_op1("__intrinsic_atomic_sub",
>>>> + v460_desktop),
>>>> + NULL);
>>>> + add_function("atomicCounterMin",
>>>> +
>>>> _atomic_counter_op1("__intrinsic_atomic_min",
>>>> + v460_desktop),
>>>> + NULL);
>>>> + add_function("atomicCounterMax",
>>>> +
>>>> _atomic_counter_op1("__intrinsic_atomic_max",
>>>> + v460_desktop),
>>>> + NULL);
>>>> + add_function("atomicCounterAnd",
>>>> +
>>>> _atomic_counter_op1("__intrinsic_atomic_and",
>>>> + v460_desktop),
>>>> + NULL);
>>>> + add_function("atomicCounterOr",
>>>> +
>>>> _atomic_counter_op1("__intrinsic_atomic_or",
>>>> + v460_desktop),
>>>> + NULL);
>>>> + add_function("atomicCounterXor",
>>>> +
>>>> _atomic_counter_op1("__intrinsic_atomic_xor",
>>>> + v460_desktop),
>>>> + NULL);
>>>> + add_function("atomicCounterExchange",
>>>> +
>>>> _atomic_counter_op1("__intrinsic_atomic_exchange",
>>>> + v460_desktop),
>>>> + NULL);
>>>> + add_function("atomicCounterCompSwap",
>>>> +
>>>> _atomic_counter_op2("__intrinsic_atomic_comp_swap",
>>>> + v460_desktop),
>>>> + NULL);
>>>> +
>>>>
>>>>
>>>> So all of these reference the __intrinsic_atomic_max
>>>> functions. Do
>>>> those ned to be fixed up too? Specifically,
>>>>
>>>> add_function("__intrinsic_atomic_max",
>>>> _atomic_intrinsic2(buffer_atomics_supported,
>>>> glsl_type::uint_type,
>>>>
>>>> ir_intrinsic_generic_atomic_max),
>>>> _atomic_intrinsic2(buffer_atomics_supported,
>>>> glsl_type::int_type,
>>>>
>>>> ir_intrinsic_generic_atomic_max),
>>>>
>>>> _atomic_counter_intrinsic1(shader_atomic_counter_ops,
>>>>
>>>> ir_intrinsic_atomic_counter_max),
>>>> NULL);
>>>>
>>>> I believe the latter one needs to have its availability
>>>> function
>>>> adjusted to be a||b.
>>>>
>>>>
>>>> Yes, but it's already adjusted in this patch:
>>>>
>>>>
>>>> @@ -483,7 +489,7 @@ shader_atomic_counters(const
>>>> _mesa_glsl_parse_state *state)
>>>> static bool
>>>> shader_atomic_counter_ops(const _mesa_glsl_parse_state *state)
>>>> {
>>>> - return state->ARB_shader_atomic_counter_ops_enable;
>>>> + return v460_desktop(state) ||
>>>> state->ARB_shader_atomic_counter_ops_enable;
>>>> }
>>>>
>>>>
>>>> Will that not make the suffixed functions available in plain
>>>> #version 460
>>>> then?
>>>
>>>
>>> Yeah, but does this really matter?
>>
>> Why not just always use "true" as the availability predicate?
>>
>> We've been pretty careful about not over-exposing stuff - I think it'd
>> be against spec to do so, and I suspect a lot of others would agree.
>> If the concensus is otherwise, we should just get rid of the
>> availability functions entirely...
>
> Hmm, actually that's a good point. When I read the patch, I was thinking
> we always have the extension when we have 460, but a #version 460 shader
> that does not request the extension should not be seeing the suffixed
> versions of those functions.
Yeah, it makes sense, I will update the patch. Thanks
Samuel.
>
> Cheers,
> Nicolai
More information about the mesa-dev
mailing list