Re: [Mesa-dev] [PATCH 08/10] glsl: A shader cannot redefine or overload built-in functions in GLSL ES 3.00

2015-02-18 Thread Samuel Iglesias Gonsálvez
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

2015-02-18 Thread Samuel Iglesias Gonsálvez
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

2015-02-18 Thread Kenneth Graunke
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

2015-02-18 Thread Matt Turner
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

2014-12-04 Thread Samuel Iglesias Gonsálvez
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

2014-12-02 Thread Chris Forbes
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

2014-12-02 Thread Ian Romanick
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