On 04.08.2017 17:46, Ilia Mirkin wrote:
On Fri, Aug 4, 2017 at 10:49 AM, Samuel Pitoiset
<samuel.pitoi...@gmail.com> wrote:



On 08/04/2017 04:27 PM, Ilia Mirkin wrote:



On Aug 4, 2017 02:02, "Samuel Pitoiset" <samuel.pitoi...@gmail.com
<mailto:samuel.pitoi...@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.pitoi...@gmail.com <mailto:samuel.pitoi...@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.pitoi...@gmail.com
             <mailto:samuel.pitoi...@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.

Cheers,
Nicolai
--
Lerne, wie die Welt wirklich ist,
Aber vergiss niemals, wie sie sein sollte.
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Reply via email to