From f81ede469910d80ef000f313e3388d65a6bd0afd Mon Sep 17 00:00:00 2001 From: Lionel Landwerlin Date: Tue, 7 Mar 2017 18:37:58 +0000 Subject: [PATCH] glsl: builtin: always return clones of the builtins Builtins are created once and allocated using their own private ralloc context. When reparenting IR that includes builtins, we might be steal bits of builtins. This is problematic because these builtins might now be freed when the shader that includes then last is disposed. This might also lead to inconsistent ralloc trees/lists if shaders are created on multiple threads. Rather than including builtins directly into a shader's IR, we should include clones of them in the ralloc context of the shader that requires them. This fixes double free issues we've been seeing when running shader-db on a big multicore (72 threads) server. v2: Also rename _mesa_glsl_find_builtin_function_by_name() to better reflect how this function is used. (Ken) v3: Rename ctx to mem_ctx (Ken) Signed-off-by: Lionel Landwerlin Reviewed-by: Kenneth Graunke --- src/compiler/glsl/ast_to_hir.cpp | 2 +- src/compiler/glsl/builtin_functions.cpp | 22 +++++++++++++++++----- src/compiler/glsl/builtin_functions.h | 4 ++-- 3 files changed, 20 insertions(+), 8 deletions(-) diff --git a/src/compiler/glsl/ast_to_hir.cpp b/src/compiler/glsl/ast_to_hir.cpp index 59d03c9c962..27dc21fffec 100644 --- a/src/compiler/glsl/ast_to_hir.cpp +++ b/src/compiler/glsl/ast_to_hir.cpp @@ -5653,7 +5653,7 @@ ast_function::hir(exec_list *instructions, if (state->es_shader && state->language_version >= 300) { /* Local shader has no exact candidates; check the built-ins. */ _mesa_glsl_initialize_builtin_functions(); - if (_mesa_glsl_find_builtin_function_by_name(name)) { + if (_mesa_glsl_has_builtin_function(name)) { YYLTYPE loc = this->get_location(); _mesa_glsl_error(& loc, state, "A shader cannot redefine or overload built-in " diff --git a/src/compiler/glsl/builtin_functions.cpp b/src/compiler/glsl/builtin_functions.cpp index e03a50c8438..e30509a4acd 100644 --- a/src/compiler/glsl/builtin_functions.cpp +++ b/src/compiler/glsl/builtin_functions.cpp @@ -62,6 +62,7 @@ #include "program/prog_instruction.h" #include #include "builtin_functions.h" +#include "util/hash_table.h" #define M_PIf ((float) M_PI) #define M_PI_2f ((float) M_PI_2) @@ -6002,21 +6003,32 @@ ir_function_signature * _mesa_glsl_find_builtin_function(_mesa_glsl_parse_state *state, const char *name, exec_list *actual_parameters) { - ir_function_signature * s; + ir_function_signature *s; mtx_lock(&builtins_lock); s = builtins.find(state, name, actual_parameters); mtx_unlock(&builtins_lock); - return s; + + if (s == NULL) + return NULL; + + struct hash_table *ht = + _mesa_hash_table_create(NULL, _mesa_hash_pointer, _mesa_key_pointer_equal); + void *mem_ctx = state; + ir_function *f = s->function()->clone(mem_ctx, ht); + _mesa_hash_table_destroy(ht, NULL); + + return f->matching_signature(state, actual_parameters, true); } -ir_function * -_mesa_glsl_find_builtin_function_by_name(const char *name) +bool +_mesa_glsl_has_builtin_function(const char *name) { ir_function *f; mtx_lock(&builtins_lock); f = builtins.shader->symbols->get_function(name); mtx_unlock(&builtins_lock); - return f; + + return f != NULL; } gl_shader * diff --git a/src/compiler/glsl/builtin_functions.h b/src/compiler/glsl/builtin_functions.h index 7ae211b48aa..14a52b94027 100644 --- a/src/compiler/glsl/builtin_functions.h +++ b/src/compiler/glsl/builtin_functions.h @@ -31,8 +31,8 @@ extern ir_function_signature * _mesa_glsl_find_builtin_function(_mesa_glsl_parse_state *state, const char *name, exec_list *actual_parameters); -extern ir_function * -_mesa_glsl_find_builtin_function_by_name(const char *name); +extern bool +_mesa_glsl_has_builtin_function(const char *name); extern gl_shader * _mesa_glsl_get_builtin_function_shader(void); -- 2.30.2