[Mesa-dev] [PATCH] linker: Only over-ride built-ins when a prototype has been seen

[Ian Romanick]

From: Ian Romanick <ian.d.romanick at intel.com>

The GLSL spec says:

    "If a built-in function is redeclared in a shader (i.e., a
    prototype is visible) before a call to it, then the linker will
    only attempt to resolve that call within the set of shaders that
    are linked with it."

This patch enforces this behavior.  When a function call is processed
a flag is set in the ir_call to indicate whether the previously seen
prototype is the built-in or not.  At link time a call will only bind
to an instance of a function that matches the "want built-in" setting
in the ir_call.

This has the odd side effect that first call to abs() in the shader
below will call the built-in and the second will not:

float foo(float x) { return abs(x); }
float abs(float x) { return -x; }
float bar(float x) { return abs(x); }

This seems insane, but it matches what the spec says.

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=31744
---
 src/glsl/ir.h               |    4 ++++
 src/glsl/link_functions.cpp |   22 +++++++++++++++++-----
 2 files changed, 21 insertions(+), 5 deletions(-)

diff --git a/src/glsl/ir.h b/src/glsl/ir.h
index 42a3936..80ad3dd 100644
--- a/src/glsl/ir.h
+++ b/src/glsl/ir.h
@@ -991,6 +991,7 @@ public:
       assert(callee->return_type != NULL);
       type = callee->return_type;
       actual_parameters->move_nodes_to(& this->actual_parameters);
+      this->use_builtin = callee->is_builtin;
    }
 
    virtual ir_call *clone(void *mem_ctx, struct hash_table *ht) const;
@@ -1054,6 +1055,9 @@ public:
    /* List of ir_rvalue of paramaters passed in this call. */
    exec_list actual_parameters;
 
+   /** Should this call only bind to a built-in function? */
+   bool use_builtin;
+
 private:
    ir_call()
       : callee(NULL)
diff --git a/src/glsl/link_functions.cpp b/src/glsl/link_functions.cpp
index ae8818b..7ba760d 100644
--- a/src/glsl/link_functions.cpp
+++ b/src/glsl/link_functions.cpp
@@ -31,7 +31,8 @@
 
 static ir_function_signature *
 find_matching_signature(const char *name, const exec_list *actual_parameters,
-			gl_shader **shader_list, unsigned num_shaders);
+			gl_shader **shader_list, unsigned num_shaders,
+			bool use_builtin);
 
 class call_link_visitor : public ir_hierarchical_visitor {
 public:
@@ -75,7 +76,8 @@ public:
        * final linked shader.  If it does, use it as the target of the call.
        */
       ir_function_signature *sig =
-	 find_matching_signature(name, &callee->parameters, &linked, 1);
+	 find_matching_signature(name, &callee->parameters, &linked, 1,
+				 ir->use_builtin);
       if (sig != NULL) {
 	 ir->set_callee(sig);
 	 return visit_continue;
@@ -85,7 +87,7 @@ public:
        * linked.  If it's not found there, return an error.
        */
       sig = find_matching_signature(name, &ir->actual_parameters, shader_list,
-				    num_shaders);
+				    num_shaders, ir->use_builtin);
       if (sig == NULL) {
 	 /* FINISHME: Log the full signature of unresolved function.
 	  */
@@ -110,7 +112,9 @@ public:
 
       ir_function_signature *linked_sig =
 	 f->exact_matching_signature(&callee->parameters);
-      if (linked_sig == NULL) {
+      if ((linked_sig == NULL)
+	  || ((linked_sig != NULL)
+	      && (linked_sig->is_builtin != ir->use_builtin))) {
 	 linked_sig = new(linked) ir_function_signature(callee->return_type);
 	 f->add_signature(linked_sig);
       }
@@ -241,7 +245,8 @@ private:
  */
 ir_function_signature *
 find_matching_signature(const char *name, const exec_list *actual_parameters,
-			gl_shader **shader_list, unsigned num_shaders)
+			gl_shader **shader_list, unsigned num_shaders,
+			bool use_builtin)
 {
    for (unsigned i = 0; i < num_shaders; i++) {
       ir_function *const f = shader_list[i]->symbols->get_function(name);
@@ -254,6 +259,13 @@ find_matching_signature(const char *name, const exec_list *actual_parameters,
       if ((sig == NULL) || !sig->is_defined)
 	 continue;
 
+      /* If this function expects to bind to a built-in function and the
+       * signature that we found isn't a built-in, keep looking.  Also keep
+       * looking if we expect a non-built-in but found a built-in.
+       */
+      if (use_builtin != sig->is_builtin)
+	    continue;
+
       return sig;
    }
 
-- 
1.7.4.4