[Mesa-dev] [PATCH 6/6] glsl: Simplify the built-in function linking code.

[Kenneth Graunke]

Previously, we stored an array of up to 16 additional shaders to link,
as well as a count of how many each shader actually needed.

Since the built-in functions rewrite, all the built-ins are stored in a
single shader.  So all we need is a boolean indicating whether a shader
needs to link against built-ins or not.

During linking, we can avoid creating the temporary array if none of the
shaders being linked need built-ins.  Otherwise, it's simply a copy of
the array that has one additional element.  This is much simpler.

This patch saves approximately 128 bytes of memory per gl_shader object.

Signed-off-by: Kenneth Graunke <kenneth at whitecape.org>
---
 src/glsl/ast_function.cpp            |  4 ++--
 src/glsl/builtin_functions.cpp       |  3 +--
 src/glsl/glsl_parser_extras.cpp      |  8 +++----
 src/glsl/glsl_parser_extras.h        |  4 +---
 src/glsl/linker.cpp                  | 42 +++++++++++++++++++-----------------
 src/mesa/main/ff_fragment_shader.cpp |  2 +-
 src/mesa/main/mtypes.h               |  4 +---
 7 files changed, 31 insertions(+), 36 deletions(-)

diff --git a/src/glsl/ast_function.cpp b/src/glsl/ast_function.cpp
index 36bb260..6423759 100644
--- a/src/glsl/ast_function.cpp
+++ b/src/glsl/ast_function.cpp
@@ -458,8 +458,8 @@ no_matching_function_error(const char *name,
 
    print_function_prototypes(state, loc, state->symbols->get_function(name));
 
-   if (state->num_builtins_to_link) {
-      gl_shader *sh = state->builtins_to_link[0];
+   if (state->uses_builtin_functions) {
+      gl_shader *sh = _mesa_glsl_get_builtin_function_shader();
       print_function_prototypes(state, loc, sh->symbols->get_function(name));
    }
 }
diff --git a/src/glsl/builtin_functions.cpp b/src/glsl/builtin_functions.cpp
index e768685..6af587a 100644
--- a/src/glsl/builtin_functions.cpp
+++ b/src/glsl/builtin_functions.cpp
@@ -609,8 +609,7 @@ builtin_builder::find(_mesa_glsl_parse_state *state,
     * that the "no matching signature" error will list potential candidates
     * from the available built-ins.
     */
-   state->builtins_to_link[0] = shader;
-   state->num_builtins_to_link = 1;
+   state->uses_builtin_functions = true;
 
    ir_function *f = shader->symbols->get_function(name);
    if (f == NULL)
diff --git a/src/glsl/glsl_parser_extras.cpp b/src/glsl/glsl_parser_extras.cpp
index f401f03..c5fd3df 100644
--- a/src/glsl/glsl_parser_extras.cpp
+++ b/src/glsl/glsl_parser_extras.cpp
@@ -76,7 +76,8 @@ _mesa_glsl_parse_state::_mesa_glsl_parse_state(struct gl_context *_ctx,
    this->loop_nesting_ast = NULL;
 
    this->struct_specifier_depth = 0;
-   this->num_builtins_to_link = 0;
+
+   this->uses_builtin_functions = false;
 
    /* Set default language version and extensions */
    this->language_version = ctx->Const.ForceGLSLVersion ?
@@ -1530,10 +1531,7 @@ _mesa_glsl_compile_shader(struct gl_context *ctx, struct gl_shader *shader,
    shader->InfoLog = state->info_log;
    shader->Version = state->language_version;
    shader->IsES = state->es_shader;
-
-   memcpy(shader->builtins_to_link, state->builtins_to_link,
-          sizeof(shader->builtins_to_link[0]) * state->num_builtins_to_link);
-   shader->num_builtins_to_link = state->num_builtins_to_link;
+   shader->uses_builtin_functions = state->uses_builtin_functions;
 
    if (shader->UniformBlocks)
       ralloc_free(shader->UniformBlocks);
diff --git a/src/glsl/glsl_parser_extras.h b/src/glsl/glsl_parser_extras.h
index d232bb3..c1f5ff5 100644
--- a/src/glsl/glsl_parser_extras.h
+++ b/src/glsl/glsl_parser_extras.h
@@ -361,9 +361,7 @@ struct _mesa_glsl_parse_state {
    /** Extensions supported by the OpenGL implementation. */
    const struct gl_extensions *extensions;
 
-   /** Shaders containing built-in functions that are used for linking. */
-   struct gl_shader *builtins_to_link[16];
-   unsigned num_builtins_to_link;
+   bool uses_builtin_functions;
 
    /**
     * For geometry shaders, size of the most recently seen input declaration
diff --git a/src/glsl/linker.cpp b/src/glsl/linker.cpp
index fac186a..a37d45c 100644
--- a/src/glsl/linker.cpp
+++ b/src/glsl/linker.cpp
@@ -1405,36 +1405,38 @@ link_intrastage_shaders(void *mem_ctx,
 					      insertion_point, true, linked);
    }
 
-   /* Resolve initializers for global variables in the linked shader.
-    */
-   unsigned num_linking_shaders = num_shaders;
-   for (unsigned i = 0; i < num_shaders; i++)
-      num_linking_shaders += shader_list[i]->num_builtins_to_link;
+   /* Check if any shader needs built-in functions. */
+   bool need_builtins = false;
+   for (unsigned i = 0; i < num_shaders; i++) {
+      if (shader_list[i]->uses_builtin_functions) {
+         need_builtins = true;
+         break;
+      }
+   }
 
-   gl_shader **linking_shaders =
-      (gl_shader **) calloc(num_linking_shaders, sizeof(gl_shader *));
+   bool ok;
+   if (need_builtins) {
+      /* Make a temporary array one larger than shader_list, which will hold
+       * the built-in function shader as well.
+       */
+      gl_shader **linking_shaders = (gl_shader **)
+         calloc(num_shaders + 1, sizeof(gl_shader *));
+      memcpy(linking_shaders, shader_list, num_shaders * sizeof(gl_shader *));
+      linking_shaders[num_shaders] = _mesa_glsl_get_builtin_function_shader();
 
-   memcpy(linking_shaders, shader_list,
-	  sizeof(linking_shaders[0]) * num_shaders);
+      ok = link_function_calls(prog, linked, linking_shaders, num_shaders + 1);
 
-   unsigned idx = num_shaders;
-   for (unsigned i = 0; i < num_shaders; i++) {
-      memcpy(&linking_shaders[idx], shader_list[i]->builtins_to_link,
-	     sizeof(linking_shaders[0]) * shader_list[i]->num_builtins_to_link);
-      idx += shader_list[i]->num_builtins_to_link;
+      free(linking_shaders);
+   } else {
+      ok = link_function_calls(prog, linked, shader_list, num_shaders);
    }
 
-   assert(idx == num_linking_shaders);
 
-   if (!link_function_calls(prog, linked, linking_shaders,
-			    num_linking_shaders)) {
+   if (!ok) {
       ctx->Driver.DeleteShader(ctx, linked);
-      free(linking_shaders);
       return NULL;
    }
 
-   free(linking_shaders);
-
    /* At this point linked should contain all of the linked IR, so
     * validate it to make sure nothing went wrong.
     */
diff --git a/src/mesa/main/ff_fragment_shader.cpp b/src/mesa/main/ff_fragment_shader.cpp
index 01edd3f..0d75702 100644
--- a/src/mesa/main/ff_fragment_shader.cpp
+++ b/src/mesa/main/ff_fragment_shader.cpp
@@ -1344,7 +1344,7 @@ create_new_program(struct gl_context *ctx, struct state_key *key)
 
    p.shader->CompileStatus = true;
    p.shader->Version = state->language_version;
-   p.shader->num_builtins_to_link = state->num_builtins_to_link;
+   p.shader->uses_builtin_functions = state->uses_builtin_functions;
    p.shader_program->Shaders =
       (gl_shader **)malloc(sizeof(*p.shader_program->Shaders));
    p.shader_program->Shaders[0] = p.shader;
diff --git a/src/mesa/main/mtypes.h b/src/mesa/main/mtypes.h
index ecfb5e0..51d6f4b 100644
--- a/src/mesa/main/mtypes.h
+++ b/src/mesa/main/mtypes.h
@@ -2352,9 +2352,7 @@ struct gl_shader
    struct exec_list *ir;
    struct glsl_symbol_table *symbols;
 
-   /** Shaders containing built-in functions that are used for linking. */
-   struct gl_shader *builtins_to_link[16];
-   unsigned num_builtins_to_link;
+   bool uses_builtin_functions;
 
    /**
     * Geometry shader state from GLSL 1.50 layout qualifiers.
-- 
1.8.4.4