[Mesa-dev] [PATCH] glsl: Ignore redundant prototypes after a function's been defined.

[Jordan Justen]

Reviewed-by: Jordan Justen <jordan.l.justen at intel.com>

On Tue, Apr 30, 2013 at 1:52 AM, Kenneth Graunke <kenneth at 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.palli at intel.com>
> Cc: Ian Romanick <idr at freedesktop.org>
> Signed-off-by: Kenneth Graunke <kenneth at 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 at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev