[Mesa-dev] [PATCH] glsl: Rebuild the symbol table without unreachable symbols

[Ian Romanick]

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

Previously we had to keep unreachable global symbols in the symbol table
because the symbol table is used during linking.  Having the symbol
table retain pointers to freed memory... what could possibly go wrong?
At the same time, this meant that we kept live references to tons of
memory that was no longer needed.

New strategy:  destroy the old symbol table, and make a new one from the
reachable symbols.

Assuming that I'm reading the output of massif correctly, the before
peak in a trimmed apitrace of dota2 on 64-bit is:

 72 52,068,527,325      104,996,296       97,108,557     7,887,739            0

and the after peak is:

 66 52,086,970,751       94,286,304       88,121,489     6,164,815            0

A real savings of 10.2MB (~10%).

Signed-off-by: Ian Romanick <ian.d.romanick at intel.com>
Cc: Kenneth Graunke <kenneth at whitecape.org>
---
 src/glsl/glsl_parser_extras.cpp | 39 ++++++++++++++++++++++++++++++++++++++-
 1 file changed, 38 insertions(+), 1 deletion(-)

diff --git a/src/glsl/glsl_parser_extras.cpp b/src/glsl/glsl_parser_extras.cpp
index 18e08e6..3d1beff 100644
--- a/src/glsl/glsl_parser_extras.cpp
+++ b/src/glsl/glsl_parser_extras.cpp
@@ -1510,7 +1510,7 @@ _mesa_glsl_compile_shader(struct gl_context *ctx, struct gl_shader *shader,
    if (shader->InfoLog)
       ralloc_free(shader->InfoLog);
 
-   shader->symbols = state->symbols;
+   shader->symbols = new(shader->ir) glsl_symbol_table;
    shader->CompileStatus = !state->error;
    shader->InfoLog = state->info_log;
    shader->Version = state->language_version;
@@ -1523,6 +1523,43 @@ _mesa_glsl_compile_shader(struct gl_context *ctx, struct gl_shader *shader,
    /* Retain any live IR, but trash the rest. */
    reparent_ir(shader->ir, shader->ir);
 
+   /* Destroy the symbol table.  Create a new symbol table that contains only
+    * the variables and functions that still exist in the IR.  The symbol
+    * table will be used later during linking.
+    *
+    * There must NOT be any freed objects still referenced by the symbol
+    * table.  That could cause the linker to dereference freed memory.
+    *
+    * We don't have to worry about types or interface-types here because those
+    * are fly-weights that are looked up by glsl_type.
+    */
+   foreach_list (node, shader->ir) {
+      ir_instruction *const ir = (ir_instruction *) node;
+
+      switch (ir->get_ir_type()) {
+      case ir_type_function: {
+         /* If the function is a built-in that is partially overridden in the
+          * shader, the ir_function stored in the symbol table may not be the
+          * same as the one that appears in the shader.  The one in the shader
+          * will only include definitions from inside the shader.  We need the
+          * one from the symbol table because it will include built-in
+          * defintions and definitions from the shader.
+          */
+         ir_function *const def = (ir_function *) ir;
+         ir_function *const decl = state->symbols->get_function(def->name);
+
+         shader->symbols->add_function(decl);
+         break;
+      }
+      case ir_type_variable:
+         shader->symbols->add_variable((ir_variable *) ir);
+         break;
+      default:
+         break;
+      }
+   }
+
+   delete state->symbols;
    ralloc_free(state);
 }
 
-- 
1.8.1.4