[Mesa-dev] [PATCH] Revert "compiler/glsl: handle case where we have multiple users for types"
Tapani Pälli
tapani.palli at intel.com
Thu Apr 18 05:08:39 UTC 2019
On 4/18/19 8:05 AM, Timothy Arceri wrote:
> This reverts commit 624789e3708c87ea2a4c8d2266266b489b421cba.
>
> It caused 400+ piglit tests to crash on radeonsi, I haven't been able
> to track down the reason yet.
Could you share the backtrace?
> ---
> src/amd/vulkan/radv_device.c | 3 ---
> src/compiler/glsl/glsl_parser_extras.cpp | 20 ++-------------
> src/compiler/glsl/glsl_parser_extras.h | 2 --
> src/compiler/glsl/standalone.cpp | 3 +--
> src/compiler/glsl_types.cpp | 32 ++++--------------------
> src/compiler/glsl_types.h | 10 +++-----
> src/intel/vulkan/anv_device.c | 3 ---
> src/mesa/main/context.c | 4 ---
> 8 files changed, 11 insertions(+), 66 deletions(-)
>
> diff --git a/src/amd/vulkan/radv_device.c b/src/amd/vulkan/radv_device.c
> index 96c6543141e..13021a9f2da 100644
> --- a/src/amd/vulkan/radv_device.c
> +++ b/src/amd/vulkan/radv_device.c
> @@ -48,7 +48,6 @@
> #include "util/build_id.h"
> #include "util/debug.h"
> #include "util/mesa-sha1.h"
> -#include "compiler/glsl_types.h"
>
> static int
> radv_device_get_cache_uuid(enum radeon_family family, void *uuid)
> @@ -583,7 +582,6 @@ VkResult radv_CreateInstance(
> }
>
> _mesa_locale_init();
> - glsl_type_singleton_init_or_ref();
>
> VG(VALGRIND_CREATE_MEMPOOL(instance, 0, false));
>
> @@ -609,7 +607,6 @@ void radv_DestroyInstance(
>
> VG(VALGRIND_DESTROY_MEMPOOL(instance));
>
> - glsl_type_singleton_decref();
> _mesa_locale_fini();
>
> vk_debug_report_instance_destroy(&instance->debug_report_callbacks);
> diff --git a/src/compiler/glsl/glsl_parser_extras.cpp b/src/compiler/glsl/glsl_parser_extras.cpp
> index b30c638cdc9..0ffad2d25a0 100644
> --- a/src/compiler/glsl/glsl_parser_extras.cpp
> +++ b/src/compiler/glsl/glsl_parser_extras.cpp
> @@ -2324,24 +2324,6 @@ do_common_optimization(exec_list *ir, bool linked,
>
> extern "C" {
>
> -/**
> - * To be called at GL context ctor.
> - */
> -void
> -_mesa_init_shader_compiler_types(void)
> -{
> - glsl_type_singleton_init_or_ref();
> -}
> -
> -/**
> - * To be called at GL context dtor.
> - */
> -void
> -_mesa_destroy_shader_compiler_types(void)
> -{
> - glsl_type_singleton_decref();
> -}
> -
> /**
> * To be called at GL teardown time, this frees compiler datastructures.
> *
> @@ -2353,6 +2335,8 @@ void
> _mesa_destroy_shader_compiler(void)
> {
> _mesa_destroy_shader_compiler_caches();
> +
> + _mesa_glsl_release_types();
> }
>
> /**
> diff --git a/src/compiler/glsl/glsl_parser_extras.h b/src/compiler/glsl/glsl_parser_extras.h
> index f92d2160aac..8646ba6cadd 100644
> --- a/src/compiler/glsl/glsl_parser_extras.h
> +++ b/src/compiler/glsl/glsl_parser_extras.h
> @@ -1010,8 +1010,6 @@ extern int glcpp_preprocess(void *ctx, const char **shader, char **info_log,
> struct _mesa_glsl_parse_state *state,
> struct gl_context *gl_ctx);
>
> -extern void _mesa_init_shader_compiler_types(void);
> -extern void _mesa_destroy_shader_compiler_types(void);
> extern void _mesa_destroy_shader_compiler(void);
> extern void _mesa_destroy_shader_compiler_caches(void);
>
> diff --git a/src/compiler/glsl/standalone.cpp b/src/compiler/glsl/standalone.cpp
> index 7b3d358ca96..942b9ee4986 100644
> --- a/src/compiler/glsl/standalone.cpp
> +++ b/src/compiler/glsl/standalone.cpp
> @@ -132,7 +132,6 @@ static void
> initialize_context(struct gl_context *ctx, gl_api api)
> {
> initialize_context_to_defaults(ctx, api);
> - glsl_type_singleton_init_or_ref();
>
> /* The standalone compiler needs to claim support for almost
> * everything in order to compile the built-in functions.
> @@ -618,6 +617,6 @@ standalone_compiler_cleanup(struct gl_shader_program *whole_program)
> delete whole_program->FragDataIndexBindings;
>
> ralloc_free(whole_program);
> - glsl_type_singleton_decref();
> + _mesa_glsl_release_types();
> _mesa_glsl_release_builtin_functions();
> }
> diff --git a/src/compiler/glsl_types.cpp b/src/compiler/glsl_types.cpp
> index 9938b3df450..66241b34281 100644
> --- a/src/compiler/glsl_types.cpp
> +++ b/src/compiler/glsl_types.cpp
> @@ -37,12 +37,6 @@ hash_table *glsl_type::interface_types = NULL;
> hash_table *glsl_type::function_types = NULL;
> hash_table *glsl_type::subroutine_types = NULL;
>
> -/* There might be multiple users for types (e.g. application using OpenGL
> - * and Vulkan simultanously or app using multiple Vulkan instances). Counter
> - * is used to make sure we don't release the types if a user is still present.
> - */
> -static uint32_t glsl_type_users = 0;
> -
> glsl_type::glsl_type(GLenum gl_type,
> glsl_base_type base_type, unsigned vector_elements,
> unsigned matrix_columns, const char *name,
> @@ -475,26 +469,12 @@ hash_free_type_function(struct hash_entry *entry)
> }
>
> void
> -glsl_type_singleton_init_or_ref()
> +_mesa_glsl_release_types(void)
> {
> - mtx_lock(&glsl_type::hash_mutex);
> - glsl_type_users++;
> - mtx_unlock(&glsl_type::hash_mutex);
> -}
> -
> -void
> -glsl_type_singleton_decref()
> -{
> - mtx_lock(&glsl_type::hash_mutex);
> -
> - assert(glsl_type_users > 0);
> -
> - /* Do not release glsl_types if they are still used. */
> - if (--glsl_type_users) {
> - mtx_unlock(&glsl_type::hash_mutex);
> - return;
> - }
> -
> + /* Should only be called during atexit (either when unloading shared
> + * object, or if process terminates), so no mutex-locking should be
> + * necessary.
> + */
> if (glsl_type::explicit_matrix_types != NULL) {
> _mesa_hash_table_destroy(glsl_type::explicit_matrix_types,
> hash_free_type_function);
> @@ -525,8 +505,6 @@ glsl_type_singleton_decref()
> _mesa_hash_table_destroy(glsl_type::subroutine_types, hash_free_type_function);
> glsl_type::subroutine_types = NULL;
> }
> -
> - mtx_unlock(&glsl_type::hash_mutex);
> }
>
>
> diff --git a/src/compiler/glsl_types.h b/src/compiler/glsl_types.h
> index 2fd93f7b945..dd9ae657019 100644
> --- a/src/compiler/glsl_types.h
> +++ b/src/compiler/glsl_types.h
> @@ -47,13 +47,10 @@ struct _mesa_glsl_parse_state;
> struct glsl_symbol_table;
>
> extern void
> -glsl_type_singleton_init_or_ref();
> -
> -extern void
> -glsl_type_singleton_decref();
> +_mesa_glsl_initialize_types(struct _mesa_glsl_parse_state *state);
>
> extern void
> -_mesa_glsl_initialize_types(struct _mesa_glsl_parse_state *state);
> +_mesa_glsl_release_types(void);
>
> void encode_type_to_blob(struct blob *blob, const struct glsl_type *type);
>
> @@ -1065,9 +1062,8 @@ private:
> * data.
> */
> /*@{*/
> - friend void glsl_type_singleton_init_or_ref(void);
> - friend void glsl_type_singleton_decref(void);
> friend void _mesa_glsl_initialize_types(struct _mesa_glsl_parse_state *);
> + friend void _mesa_glsl_release_types(void);
> /*@}*/
> };
>
> diff --git a/src/intel/vulkan/anv_device.c b/src/intel/vulkan/anv_device.c
> index 74d4eebebc2..2687c4699ca 100644
> --- a/src/intel/vulkan/anv_device.c
> +++ b/src/intel/vulkan/anv_device.c
> @@ -41,7 +41,6 @@
> #include "git_sha1.h"
> #include "vk_util.h"
> #include "common/gen_defines.h"
> -#include "compiler/glsl_types.h"
>
> #include "genxml/gen7_pack.h"
>
> @@ -717,7 +716,6 @@ VkResult anv_CreateInstance(
> env_var_as_boolean("ANV_ENABLE_PIPELINE_CACHE", true);
>
> _mesa_locale_init();
> - glsl_type_singleton_init_or_ref();
>
> VG(VALGRIND_CREATE_MEMPOOL(instance, 0, false));
>
> @@ -748,7 +746,6 @@ void anv_DestroyInstance(
>
> vk_debug_report_instance_destroy(&instance->debug_report_callbacks);
>
> - glsl_type_singleton_decref();
> _mesa_locale_fini();
>
> vk_free(&instance->alloc, instance);
> diff --git a/src/mesa/main/context.c b/src/mesa/main/context.c
> index 7300d2f3c46..e5a89d9c2fc 100644
> --- a/src/mesa/main/context.c
> +++ b/src/mesa/main/context.c
> @@ -1196,8 +1196,6 @@ _mesa_initialize_context(struct gl_context *ctx,
> /* misc one-time initializations */
> one_time_init(ctx);
>
> - _mesa_init_shader_compiler_types();
> -
> /* Plug in driver functions and context pointer here.
> * This is important because when we call alloc_shared_state() below
> * we'll call ctx->Driver.NewTextureObject() to create the default
> @@ -1385,8 +1383,6 @@ _mesa_free_context_data( struct gl_context *ctx )
>
> free(ctx->VersionString);
>
> - _mesa_destroy_shader_compiler_types();
> -
> /* unbind the context if it's currently bound */
> if (ctx == _mesa_get_current_context()) {
> _mesa_make_current(NULL, NULL, NULL);
>
More information about the mesa-dev
mailing list