On Wednesday, March 8, 2017 1:15:04 PM PST Lionel Landwerlin wrote:
> 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)
>
> Signed-off-by: Lionel Landwerlin <lionel.g.landwer...@intel.com>
> ---
> 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 59d03c9c96..27dc21fffe 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 e03a50c843..8278c51992 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 <math.h>
> #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 *ctx = state;
Let's call this mem_ctx or just pass in state directly. We originally
used "ctx" when developing the compiler out of tree, but now that we're
in Mesa where "ctx" refers to gl_context, we've tried to switch over.
Really awesome work finding this! Thanks!
Reviewed-by: Kenneth Graunke <kenn...@whitecape.org>
> + ir_function *f = s->function()->clone(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 7ae211b48a..14a52b9402 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.11.0
>
signature.asc
Description: This is a digitally signed message part.
_______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
