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

[Timothy Arceri]

On 04/07/17 08:11, Ilia Mirkin wrote:
> Yeah I'll add a compiler test.

Thanks :)

Reviewed-by: Timothy Arceri <tarceri at itsqueeze.com>

> 
> 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);
>>>
>>