[Mesa-dev] [PATCH 2/3] glsl: Only create one ir_function for a given name.

[Kenneth Graunke]

Piglit's spec/glsl-1.10/linker/override-builtin-{const,uniform}-05 tests
do the following:

1. Call abs(float) - a built-in function.
2. Create a user-defined replacement for abs(float).
3. Call abs(float) again - now the user function.

At step 1, we created an ir_function which included the built-in
signature, added it to the symbol table, and emitted it into the IR
stream.

Then, when processing the function definition at step 2, we'd see that
there was already an ir_function.  But, since there were no user-defined
functions, we skipped over a bunch of code, and ended up creating a
second one.  This new ir_function shadowed the original in the symbol
table, but both ended up in the IR stream.

This results in an awkward situation where searching for an ir_function
via the symbol table, a forward linked list walk, and a reverse linked
list walk may return different ir_functions.  This seems undesirable.

This patch instead re-uses the existing ir_function, putting both
built-in and user-defined signatures in the same one.  The previous
patch's additional filtering ensures everything continues working.

Signed-off-by: Kenneth Graunke <kenneth at whitecape.org>
---
 src/glsl/ast_to_hir.cpp | 31 +++++++++++++++++--------------
 1 file changed, 17 insertions(+), 14 deletions(-)

diff --git a/src/glsl/ast_to_hir.cpp b/src/glsl/ast_to_hir.cpp
index a15ee9c..499b6da 100644
--- a/src/glsl/ast_to_hir.cpp
+++ b/src/glsl/ast_to_hir.cpp
@@ -4110,12 +4110,27 @@ ast_function::hir(exec_list *instructions,
                        name);
    }
 
+   /* Create an ir_function if one doesn't already exist. */
+   f = state->symbols->get_function(name);
+   if (f == NULL) {
+      f = new(ctx) ir_function(name);
+      if (!state->symbols->add_function(f)) {
+         /* This function name shadows a non-function use of the same name. */
+         YYLTYPE loc = this->get_location();
+
+         _mesa_glsl_error(&loc, state, "function name `%s' conflicts with "
+                          "non-function", name);
+         return NULL;
+      }
+
+      emit_function(state, f);
+   }
+
    /* Verify that this function's signature either doesn't match a previously
     * seen signature for a function with the same name, or, if a match is found,
     * that the previously seen signature does not have an associated definition.
     */
-   f = state->symbols->get_function(name);
-   if (f != NULL && (state->es_shader || f->has_user_signature())) {
+   if (state->es_shader || f->has_user_signature()) {
       sig = f->exact_matching_signature(state, &hir_parameters);
       if (sig != NULL) {
          const char *badvar = sig->qualifiers_match(&hir_parameters);
@@ -4146,18 +4161,6 @@ ast_function::hir(exec_list *instructions,
             }
          }
       }
-   } else {
-      f = new(ctx) ir_function(name);
-      if (!state->symbols->add_function(f)) {
-         /* This function name shadows a non-function use of the same name. */
-         YYLTYPE loc = this->get_location();
-
-         _mesa_glsl_error(&loc, state, "function name `%s' conflicts with "
-                          "non-function", name);
-         return NULL;
-      }
-
-      emit_function(state, f);
    }
 
    /* Verify the return type of main() */
-- 
2.0.2