Re: [Mesa-dev] [PATCH 08/10] glsl: A shader cannot redefine or overload built-in functions in GLSL ES 3.00
On Wednesday 18 February 2015 12:00:39 Matt Turner wrote: > On Mon, Dec 1, 2014 at 5:04 AM, Eduardo Lima Mitev wrote: > > From: Samuel Iglesias Gonsalvez > > > > 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 > > --- > > > > 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 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
Re: [Mesa-dev] [PATCH 08/10] glsl: A shader cannot redefine or overload built-in functions in GLSL ES 3.00
On Wednesday 18 February 2015 15:08:19 Kenneth Graunke wrote: > On Wednesday, February 18, 2015 12:00:39 PM Matt Turner wrote: > > On Mon, Dec 1, 2014 at 5:04 AM, Eduardo Lima Mitev wrote: > > > From: Samuel Iglesias Gonsalvez > > > > > > 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_functio > > > n_vertex > > > dEQP-GLES3.functional.shaders.functions.invalid.overload_builtin_functi > > > on_fragment > > > dEQP-GLES3.functional.shaders.functions.invalid.redefine_builtin_functi > > > on_vertex > > > dEQP-GLES3.functional.shaders.functions.invalid.redefine_builtin_functi > > > on_fragment > > > > > > No piglit regressions. > > > > > > Signed-off-by: Samuel Iglesias Gonsalvez > > > --- > > > > > > 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. > > The GLSL ES 1.0 specification actually very explicitly allows > overloading of built-in functions: > > From the GLSL ES 1.0 specification, section 4.2.6: > "User defined functions may overload built-in functions." > and chapter 8: > "User code can overload the built-in functions but cannot redefine > them." > > This is indeed different than GLSL ES 3.00, which prohibits both, > and different than desktop GLSL, which has entirely different rules. > > > > + * > > > + */ > > > + 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); +} > > > + } > > This code looks good, but there's one subtle issue with where you've > placed it. Namely, it allows this to compile: > > #version 300 es > precision highp float; > out vec4 color; > > void main() > { >color = vec4(sin(0.5));
Re: [Mesa-dev] [PATCH 08/10] glsl: A shader cannot redefine or overload built-in functions in GLSL ES 3.00
On Wednesday, February 18, 2015 12:00:39 PM Matt Turner wrote: > On Mon, Dec 1, 2014 at 5:04 AM, Eduardo Lima Mitev wrote: > > From: Samuel Iglesias Gonsalvez > > > > 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 > > --- > > 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. The GLSL ES 1.0 specification actually very explicitly allows overloading of built-in functions: From the GLSL ES 1.0 specification, section 4.2.6: "User defined functions may overload built-in functions." and chapter 8: "User code can overload the built-in functions but cannot redefine them." This is indeed different than GLSL ES 3.00, which prohibits both, and different than desktop GLSL, which has entirely different rules. > > > + * > > + */ > > + 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); > > +} > > + } This code looks good, but there's one subtle issue with where you've placed it. Namely, it allows this to compile: #version 300 es precision highp float; out vec4 color; void main() { color = vec4(sin(0.5)); } float sin(float x); When handling that prototype, it will take the existing exact_matching_signature != NULL path (since your code is the "else" to that block), which sees it as a harmless redundant prototype, and allows it. I would advise simply moving this code out a level and up: if (state->es_shader && state->language_version >= 300) { /
Re: [Mesa-dev] [PATCH 08/10] glsl: A shader cannot redefine or overload built-in functions in GLSL ES 3.00
On Mon, Dec 1, 2014 at 5:04 AM, Eduardo Lima Mitev wrote: > From: Samuel Iglesias Gonsalvez > > 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 > --- > 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 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 08/10] glsl: A shader cannot redefine or overload built-in functions in GLSL ES 3.00
On Wednesday, December 03, 2014 10:09:59 AM Chris Forbes wrote: > It would be nice if this could be added to the existing logic for the > interaction between builtins and app-provided overloads -- or do we > need to fail earlier than that? > This is the place where we store the information about a new function in its signature, checking that it doesn't redefine another shader-provided function and other related checks. It maybe makes more sense to move this change up in the function. So we detect here, before storing such information, if a shader is redefining or overloading a built-in function in GLSL ES 3.0. Or am I missing something? Which place do you think is better to put this? Sam > - Chris > > On Tue, Dec 2, 2014 at 2:04 AM, Eduardo Lima Mitev wrote: > > From: Samuel Iglesias Gonsalvez > > > > 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 > > --- > > > > 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." > > + * > > + */ > > + 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; > > + mtx_lock(&builtins_lock); > > + f = builtins.shader->symbols->get_function(name); > > + mtx_unlock(&builtins_lock); > > + return f; > > +} > > + > > > > gl_shader * > > _mesa_glsl_g
Re: [Mesa-dev] [PATCH 08/10] glsl: A shader cannot redefine or overload built-in functions in GLSL ES 3.00
It would be nice if this could be added to the existing logic for the interaction between builtins and app-provided overloads -- or do we need to fail earlier than that? - Chris On Tue, Dec 2, 2014 at 2:04 AM, Eduardo Lima Mitev wrote: > From: Samuel Iglesias Gonsalvez > > 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 > --- > 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." > + * > + */ > + 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; > + mtx_lock(&builtins_lock); > + f = builtins.shader->symbols->get_function(name); > + mtx_unlock(&builtins_lock); > + return f; > +} > + > gl_shader * > _mesa_glsl_get_builtin_function_shader() > { > diff --git a/src/glsl/ir.h b/src/glsl/ir.h > index a0f48b2..f2d8269 100644 > --- a/src/glsl/ir.h > +++ b/src/glsl/ir.h > @@ -2417,6 +2417,10 @@ extern ir_function_signature * > _mesa_glsl_find_builtin_function(_mesa_glsl_parse_state *state, > const char *name, exec_list > *actual_parameters); > > +extern ir_function * > +_mesa_glsl_find_builtin_function_by_name(_mesa_glsl_parse_state *state, > + const char *name); > + > extern gl_shader * > _mesa_glsl_get_builtin_function_shader(void); > > -- > 2.1.3 > > ___ > mesa-dev mailing list > mesa-dev@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/mesa-dev ___ m
Re: [Mesa-dev] [PATCH 08/10] glsl: A shader cannot redefine or overload built-in functions in GLSL ES 3.00
Oof. I'd really like to have Ken review this code. He's the expert in this area... On 12/01/2014 05:04 AM, Eduardo Lima Mitev wrote: > From: Samuel Iglesias Gonsalvez > > 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 > --- > 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." > + * > + */ > + 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; > + mtx_lock(&builtins_lock); > + f = builtins.shader->symbols->get_function(name); > + mtx_unlock(&builtins_lock); > + return f; > +} > + > gl_shader * > _mesa_glsl_get_builtin_function_shader() > { > diff --git a/src/glsl/ir.h b/src/glsl/ir.h > index a0f48b2..f2d8269 100644 > --- a/src/glsl/ir.h > +++ b/src/glsl/ir.h > @@ -2417,6 +2417,10 @@ extern ir_function_signature * > _mesa_glsl_find_builtin_function(_mesa_glsl_parse_state *state, > const char *name, exec_list > *actual_parameters); > > +extern ir_function * > +_mesa_glsl_find_builtin_function_by_name(_mesa_glsl_parse_state *state, > + const char *name); > + > extern gl_shader * > _mesa_glsl_get_builtin_function_shader(void); > > ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev