[Mesa-dev] [PATCH 2/3] glsl: clone builtin function constants

[Timothy Arceri]

f81ede469910d fixed a problem with shaders including IR that was
owned by builtins. However the approach of cloning the whole
function each time we referenced it lead to a significant
reduction in the GLSL IR compiler performance.

Everything was already cloned when inlining the function, as
far as I can tell this is the only place where we are grabbing
IR owned by the builtins without cloning it.

The double free can be easily reproduced by compiling the
Deus Ex: Mankind Divided shaders with shader db, and then
compiling them again after deleting mesa's shader cache
index file. This will cause optimisations to never be performed
on the IR and which presumably caused this issue to be hidden
under normal circumstances.

Cc: Kenneth Graunke <kenneth at whitecape.org>
Cc: Lionel Landwerlin <lionel.g.landwerlin at intel.com>
---
 src/compiler/glsl/ast_function.cpp | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/src/compiler/glsl/ast_function.cpp b/src/compiler/glsl/ast_function.cpp
index f7e90fba5b..73c4c0df7b 100644
--- a/src/compiler/glsl/ast_function.cpp
+++ b/src/compiler/glsl/ast_function.cpp
@@ -514,21 +514,29 @@ generate_call(exec_list *instructions, ir_function_signature *sig,
     *           return 0 when evaluated inside an initializer with an argument
     *           that is a constant expression."
     *
     * If the function call is a constant expression, don't generate any
     * instructions; just generate an ir_constant.
     */
    if (state->is_version(120, 100)) {
       ir_constant *value = sig->constant_expression_value(actual_parameters,
                                                           NULL);
       if (value != NULL) {
-         return value;
+         if (sig->is_builtin()) {
+            /* This value belongs to a builtin so we must clone it to avoid
+             * race conditions when freeing shaders and destorying the
+             * context.
+             */
+            return value->clone(ctx, NULL);
+         } else {
+            return value;
+         }
       }
    }
 
    ir_dereference_variable *deref = NULL;
    if (!sig->return_type->is_void()) {
       /* Create a new temporary to hold the return value. */
       char *const name = ir_variable::temporaries_allocate_names
          ? ralloc_asprintf(ctx, "%s_retval", sig->function_name())
          : NULL;
 
-- 
2.13.3