[Mesa-dev] [PATCH v4] glsl: update the extensions that are enabled for 460

Ilia Mirkin imirkin at alum.mit.edu
Fri Aug 4 15:46:14 UTC 2017


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...


More information about the mesa-dev mailing list