Reviewed-by: Jordan Justen <jordan.l.jus...@intel.com>
On Tue, Apr 30, 2013 at 1:52 AM, Kenneth Graunke <kenn...@whitecape.org> wrote:
> Consider the following shader:
>
> vec4 f(vec4 v) { return v; }
> vec4 f(vec4 v);
>
> The prototype exactly matches the signature of the earlier definition,
> so there's absolutely no point in it. However, it doesn't appear to
> be illegal. The GLSL 4.30 specification offers two relevant quotes:
>
> "If a function name is declared twice with the same parameter types,
> then the return types and all qualifiers must also match, and it is the
> same function being declared."
>
> "User-defined functions can have multiple declarations, but only one
> definition."
>
> In this case the same function was declared twice, and there's only one
> definition, which fits both pieces of text. There doesn't appear to be
> any text saying late prototypes are illegal, so presumably it's valid.
>
> Unfortunately, it currently triggers an assertion failure:
> ir_dereference_variable @ <p1> specifies undeclared variable `v' @ <p2>
>
> When we process the second line, we look for an existing exact match so
> we can enforce the one-definition rule. We then leave sig set to that
> existing function, and hit sig->replace_parameters(&hir_parameters),
> unfortunately nuking our existing definition's parameters (which have
> actual dereferences) with the prototype's bogus unused parameters.
>
> Simply bailing out and ignoring such late prototypes is the safest
> thing to do.
>
> Fixes Piglit's late-proto.vert as well as 3DMark/Ice Storm for Android.
>
> NOTE: This is a candidate for stable branches.
> Cc: Tapani Pälli <tapani.pa...@intel.com>
> Cc: Ian Romanick <i...@freedesktop.org>
> Signed-off-by: Kenneth Graunke <kenn...@whitecape.org>
> ---
> src/glsl/ast_to_hir.cpp | 15 +++++++++++----
> 1 file changed, 11 insertions(+), 4 deletions(-)
>
> diff --git a/src/glsl/ast_to_hir.cpp b/src/glsl/ast_to_hir.cpp
> index 2638411..e595110 100644
> --- a/src/glsl/ast_to_hir.cpp
> +++ b/src/glsl/ast_to_hir.cpp
> @@ -3205,10 +3205,17 @@ ast_function::hir(exec_list *instructions,
> "match prototype", name);
> }
>
> - if (is_definition && sig->is_defined) {
> - YYLTYPE loc = this->get_location();
> -
> - _mesa_glsl_error(& loc, state, "function `%s' redefined", name);
> + if (sig->is_defined) {
> + if (is_definition) {
> + YYLTYPE loc = this->get_location();
> + _mesa_glsl_error(& loc, state, "function `%s' redefined",
> name);
> + } else {
> + /* We just encountered a prototype that exactly matches a
> + * function that's already been defined. This is redundant,
> + * and we should ignore it.
> + */
> + return NULL;
> + }
> }
> }
> } else {
> --
> 1.8.2.1
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
