[Mesa-stable] [Mesa-dev] [PATCH] mesa: check if any of the named builtins are available first

[Ilia Mirkin]

Yeah I'll add a compiler test.

On Mon, Jul 3, 2017 at 6:02 PM, Timothy Arceri <tarceri at itsqueeze.com> wrote:
> This seems reasonable, but are there some piglit tests to go with this? I
> didn't see any on the list. At least one test for this would be nice.
>
>
> On 04/07/17 07:08, Ilia Mirkin wrote:
>>
>> _mesa_glsl_has_builtin_function is used to determine whether any variant
>> of a builtin are available, for the purpose of enforcing the GLSL ES
>> 3.00+ rule that overloads or overrides of builtins are disallowed.
>>
>> However the builtin_builder contains information on all builtins,
>> irrespective of parse state, or versions, or extension enablement. As a
>> result we would say that a builtin existed even if it was not actually
>> available.
>>
>> To resolve this, first check if at least one signature is available for
>> a builtin before returning true.
>>
>> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=101666
>> Signed-off-by: Ilia Mirkin <imirkin at alum.mit.edu>
>> Cc: mesa-stable at lists.freedesktop.org
>> ---
>>   src/compiler/glsl/ast_to_hir.cpp        |  2 +-
>>   src/compiler/glsl/builtin_functions.cpp | 13 +++++++++++--
>>   src/compiler/glsl/builtin_functions.h   |  3 ++-
>>   3 files changed, 14 insertions(+), 4 deletions(-)
>>
>> diff --git a/src/compiler/glsl/ast_to_hir.cpp
>> b/src/compiler/glsl/ast_to_hir.cpp
>> index b1e490eefd..c338ad79ca 100644
>> --- a/src/compiler/glsl/ast_to_hir.cpp
>> +++ b/src/compiler/glsl/ast_to_hir.cpp
>> @@ -5916,7 +5916,7 @@ ast_function::hir(exec_list *instructions,
>>         /* Local shader has no exact candidates; check the built-ins. */
>>         _mesa_glsl_initialize_builtin_functions();
>>         if (state->language_version >= 300 &&
>> -          _mesa_glsl_has_builtin_function(name)) {
>> +          _mesa_glsl_has_builtin_function(state, name)) {
>>            YYLTYPE loc = this->get_location();
>>            _mesa_glsl_error(& loc, state,
>>                             "A shader cannot redefine or overload built-in
>> "
>> diff --git a/src/compiler/glsl/builtin_functions.cpp
>> b/src/compiler/glsl/builtin_functions.cpp
>> index cc1432197b..84833bdd7d 100644
>> --- a/src/compiler/glsl/builtin_functions.cpp
>> +++ b/src/compiler/glsl/builtin_functions.cpp
>> @@ -6227,14 +6227,23 @@
>> _mesa_glsl_find_builtin_function(_mesa_glsl_parse_state *state,
>>   }
>>     bool
>> -_mesa_glsl_has_builtin_function(const char *name)
>> +_mesa_glsl_has_builtin_function(_mesa_glsl_parse_state *state, const char
>> *name)
>>   {
>>      ir_function *f;
>> +   bool ret = false;
>>      mtx_lock(&builtins_lock);
>>      f = builtins.shader->symbols->get_function(name);
>> +   if (f != NULL) {
>> +      foreach_in_list(ir_function_signature, sig, &f->signatures) {
>> +         if (sig->is_builtin_available(state)) {
>> +            ret = true;
>> +            break;
>> +         }
>> +      }
>> +   }
>>      mtx_unlock(&builtins_lock);
>>   -   return f != NULL;
>> +   return ret;
>>   }
>>     gl_shader *
>> diff --git a/src/compiler/glsl/builtin_functions.h
>> b/src/compiler/glsl/builtin_functions.h
>> index 14a52b9402..2053c82b74 100644
>> --- a/src/compiler/glsl/builtin_functions.h
>> +++ b/src/compiler/glsl/builtin_functions.h
>> @@ -32,7 +32,8 @@ _mesa_glsl_find_builtin_function(_mesa_glsl_parse_state
>> *state,
>>                                    const char *name, exec_list
>> *actual_parameters);
>>     extern bool
>> -_mesa_glsl_has_builtin_function(const char *name);
>> +_mesa_glsl_has_builtin_function(_mesa_glsl_parse_state *state,
>> +                                const char *name);
>>     extern gl_shader *
>>   _mesa_glsl_get_builtin_function_shader(void);
>>
>