On Wednesday 18 February 2015 12:00:39 Matt Turner wrote:
> On Mon, Dec 1, 2014 at 5:04 AM, Eduardo Lima Mitev <el...@igalia.com> wrote:
> > From: Samuel Iglesias Gonsalvez <sigles...@igalia.com>
> >
> > Create a new search function to look for matching built-in functions by
> > name and use it for built-in function redefinition or overload in GLSL ES
> > 3.00.
> >
> > GLSL ES 3.0 spec, chapter 6.1 "Function Definitions", page 71
> >
> > "A shader cannot redefine or overload built-in functions."
> >
> > In case of GLSL ES 1.0 spec, chapter 6.1 "Function Definitions", page 54
> >
> > "Function names can be overloaded. This allows the same function name to
> > be
> > used for multiple functions, as long as the argument list types differ. If
> > functions’ names and argument types match, then their return type and
> > parameter qualifiers must also match. Function signature matching is based
> > on parameter type only, no qualifiers are used. Overloading is used
> > heavily in the built-in functions. When overloaded functions (or indeed
> > any functions) are resolved, an exact match for the function's signature
> > is sought. This includes exact match of array size as well. No promotion
> > or demotion of the return type or input argument types is done. All
> > expected combination of inputs and outputs must be defined as separate
> > functions."
> >
> > So this check is specific to GLSL ES 3.00.
> >
> > This patch fixes the following dEQP tests:
> >
> > dEQP-GLES3.functional.shaders.functions.invalid.overload_builtin_function_
> > vertex
> > dEQP-GLES3.functional.shaders.functions.invalid.overload_builtin_function
> > _fragment
> > dEQP-GLES3.functional.shaders.functions.invalid.redefine_builtin_function
> > _vertex
> > dEQP-GLES3.functional.shaders.functions.invalid.redefine_builtin_function
> > _fragment
> >
> > No piglit regressions.
> >
> > Signed-off-by: Samuel Iglesias Gonsalvez <sigles...@igalia.com>
> > ---
> >
> > src/glsl/ast_to_hir.cpp | 22 ++++++++++++++++++++++
> > src/glsl/builtin_functions.cpp | 11 +++++++++++
> > src/glsl/ir.h | 4 ++++
> > 3 files changed, 37 insertions(+)
> >
> > diff --git a/src/glsl/ast_to_hir.cpp b/src/glsl/ast_to_hir.cpp
> > index fe1e129..b7074bc 100644
> > --- a/src/glsl/ast_to_hir.cpp
> > +++ b/src/glsl/ast_to_hir.cpp
> > @@ -4167,6 +4167,28 @@ ast_function::hir(exec_list *instructions,
> >
> > return NULL;
> >
> > }
> >
> > }
> >
> > + } else {
> > + /* From GLSL ES 3.0 spec, chapter 6.1 "Function Definitions",
> > page 71: + *
> > + * "A shader cannot redefine or overload built-in functions."
> > + *
> > + * While in GLSL ES 1.0 spec, chapter 6.1 "Function
> > Definitions", page + * 54, this is allowed:
> > + *
> > + * "Function names can be overloaded. [...] Overloading is used
> > heavily + * in the built-in functions."
>
> I don't think that quote is really explicitly saying that you can
> overload built-in functions. It's just saying that built-in functions
> contain many overloads.
>
> It doesn't, however, prohibit the user from adding overloads of their own.
>
> I'd probably replace the spec citation with just a statement that the
> GLSL ES 1.0 spec doesn't prohibit overloading built-in functions,
> since it really doesn't say anything explicitly about the topic.
>
> > + *
> > + */
> > + if (state->es_shader && state->language_version >= 300) {
> > + /* Local shader has no exact candidates; check the built-ins.
> > */ + _mesa_glsl_initialize_builtin_functions();
> > + if (_mesa_glsl_find_builtin_function_by_name(state, name)) {
> > + YYLTYPE loc = this->get_location();
> > + _mesa_glsl_error(& loc, state,
> > + "A shader cannot redefine or overload
> > built-in " + "function `%s' in GLSL ES
> > 3.00", name); + }
> > + }
> >
> > }
> >
> > }
> >
> > diff --git a/src/glsl/builtin_functions.cpp
> > b/src/glsl/builtin_functions.cpp index bb7fbcd..f5052d3 100644
> > --- a/src/glsl/builtin_functions.cpp
> > +++ b/src/glsl/builtin_functions.cpp
> > @@ -4618,6 +4618,17 @@
> > _mesa_glsl_find_builtin_function(_mesa_glsl_parse_state *state,>
> > return s;
> >
> > }
> >
> > +ir_function *
> > +_mesa_glsl_find_builtin_function_by_name(_mesa_glsl_parse_state *state,
> > + const char *name)
> > +{
> > + ir_function * f;
>
> Remove the space after the *.
>
> As far as I can tell, this patch looks correct. Confirmation from Ken
> would be nice as well.
>
> (This behavior should have been the default in all versions of GLSL as
> far as I'm concerned)
>
> Reviewed-by: Matt Turner <matts...@gmail.com>Following Kenneth's comments, I will send an updated version of it for review. Also, I will modify the GLSL ES 1.0 spec citation following what Kenneth says in his reply. Because of these changes, I am not going to add your R-b yet. Thanks for your review! Sam
signature.asc
Description: This is a digitally signed message part.
_______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
