mesa/compiler: rework tear down of builtin/types

The issue we're running into when running CTS is that glsl types are
deleted while builtins depending on them are not.

This happens because on one hand we have glsl types ref counted, but
builtins are not. Instead builtins are destroyed when unloading libGL
or explicitly calling glReleaseShaderCompiler().

This change removes almost entirely any dealing with glsl types
ref/unref by letting the builtins deal with it instead. In turn we
introduce a builtin ref count mechanism. Each GL context takes a
reference on the builtins when compiling a shader for the first time.
It releases the reference when the context is destroyed. It can also
explicitly release those when glReleaseShaderCompiler() is called.

Finally we also take a reference on the glsl types when loading libGL
to avoid recreating glsl types too often.

v2: Ensure we take a reference if we don't have one in link step (Lionel)

Signed-off-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=110796
Reviewed-by: Eric Anholt <eric@anholt.net>
Reviewed-by: Tapani Pälli <tapani.palli@intel.com>

diff --git a/src/compiler/glsl/ast_function.cpp b/src/compiler/glsl/ast_function.cpp
index 0ed47660e2239f7a43ebff22e4c6dc7897c71b5e..4fd9b87955560244da2c8cfde6cac9104f8c6417 100644 (file)
--- a/src/compiler/glsl/ast_function.cpp
+++ b/src/compiler/glsl/ast_function.cpp
@@ -664,7 +664,6 @@ match_function_by_name(const char *name,
    }
 
    /* Local shader has no exact candidates; check the built-ins. */
-   _mesa_glsl_initialize_builtin_functions();
    sig = _mesa_glsl_find_builtin_function(state, name, actual_parameters);
 
    /* if _mesa_glsl_find_builtin_function failed, fall back to the result

diff --git a/src/compiler/glsl/ast_to_hir.cpp b/src/compiler/glsl/ast_to_hir.cpp
index 61351341703c6ecd20fa68d41fff70ba38faa768..37afb8bd0b02870e9963bb665b8f96fbff227dd1 100644 (file)
--- a/src/compiler/glsl/ast_to_hir.cpp
+++ b/src/compiler/glsl/ast_to_hir.cpp
@@ -6043,7 +6043,6 @@ ast_function::hir(exec_list *instructions,
     */
    if (state->es_shader) {
       /* Local shader has no exact candidates; check the built-ins. */
-      _mesa_glsl_initialize_builtin_functions();
       if (state->language_version >= 300 &&
           _mesa_glsl_has_builtin_function(state, name)) {
          YYLTYPE loc = this->get_location();

diff --git a/src/compiler/glsl/builtin_functions.cpp b/src/compiler/glsl/builtin_functions.cpp
index 95d45033a01c9f0e6fa33a9baf4529e95d5472c3..eef5737128d8f485990a52b8e2dc7a4ec049de8e 100644 (file)
--- a/src/compiler/glsl/builtin_functions.cpp
+++ b/src/compiler/glsl/builtin_functions.cpp
@@ -1254,6 +1254,8 @@ builtin_builder::initialize()
    if (mem_ctx != NULL)
       return;
 
+   glsl_type_singleton_init_or_ref();
+
    mem_ctx = ralloc_context(NULL);
    create_shader();
    create_intrinsics();
@@ -1268,6 +1270,8 @@ builtin_builder::release()
 
    ralloc_free(shader);
    shader = NULL;
+
+   glsl_type_singleton_decref();
 }
 
 void
@@ -7277,24 +7281,28 @@ builtin_builder::_vote(const char *intrinsic_name,
 /* The singleton instance of builtin_builder. */
 static builtin_builder builtins;
 static mtx_t builtins_lock = _MTX_INITIALIZER_NP;
+static uint32_t builtin_users = 0;
 
 /**
  * External API (exposing the built-in module to the rest of the compiler):
  *  @{
  */
-void
-_mesa_glsl_initialize_builtin_functions()
+extern "C" void
+_mesa_glsl_builtin_functions_init_or_ref()
 {
    mtx_lock(&builtins_lock);
-   builtins.initialize();
+   if (builtin_users++ == 0)
+      builtins.initialize();
    mtx_unlock(&builtins_lock);
 }
 
-void
-_mesa_glsl_release_builtin_functions()
+extern "C" void
+_mesa_glsl_builtin_functions_decref()
 {
    mtx_lock(&builtins_lock);
-   builtins.release();
+   assert(builtin_users != 0);
+   if (--builtin_users == 0)
+      builtins.release();
    mtx_unlock(&builtins_lock);
 }
 

diff --git a/src/compiler/glsl/builtin_functions.h b/src/compiler/glsl/builtin_functions.h
index 50f4f3b14948dc16ba1db073c62d25d7011d921b..ff3d4e9f436311878191a43f8a01100dbd282ccf 100644 (file)
--- a/src/compiler/glsl/builtin_functions.h
+++ b/src/compiler/glsl/builtin_functions.h
@@ -26,8 +26,19 @@
 
 struct gl_shader;
 
-extern void
-_mesa_glsl_initialize_builtin_functions();
+#ifdef __cplusplus
+extern "C" {
+#endif
+
+void
+_mesa_glsl_builtin_functions_init_or_ref();
+
+void
+_mesa_glsl_builtin_functions_decref(void);
+
+#ifdef __cplusplus
+
+} /* extern "C" */
 
 extern ir_function_signature *
 _mesa_glsl_find_builtin_function(_mesa_glsl_parse_state *state,
@@ -43,9 +54,6 @@ _mesa_glsl_get_builtin_function_shader(void);
 extern ir_function_signature *
 _mesa_get_main_function_signature(glsl_symbol_table *symbols);
 
-extern void
-_mesa_glsl_release_builtin_functions(void);
-
 namespace generate_ir {
 
 ir_function_signature *
@@ -71,4 +79,6 @@ udivmod64(void *mem_ctx, builtin_available_predicate avail);
 
 }
 
+#endif /* __cplusplus */
+
 #endif /* BULITIN_FUNCTIONS_H */

diff --git a/src/compiler/glsl/glsl_parser_extras.cpp b/src/compiler/glsl/glsl_parser_extras.cpp
index e4a7e3dbf707bf3047b0db8d1005c779032298db..3ee1af57d506bcde1a219b73ec3c84ead22d08f6 100644 (file)
--- a/src/compiler/glsl/glsl_parser_extras.cpp
+++ b/src/compiler/glsl/glsl_parser_extras.cpp
@@ -2326,49 +2326,3 @@ do_common_optimization(exec_list *ir, bool linked,
 
    return progress;
 }
-
-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.
- *
- * After calling this, any previously compiled shaders and shader
- * programs would be invalid.  So this should happen at approximately
- * program exit.
- */
-void
-_mesa_destroy_shader_compiler(void)
-{
-   _mesa_destroy_shader_compiler_caches();
-}
-
-/**
- * Releases compiler caches to trade off performance for memory.
- *
- * Intended to be used with glReleaseShaderCompiler().
- */
-void
-_mesa_destroy_shader_compiler_caches(void)
-{
-   _mesa_glsl_release_builtin_functions();
-}
-
-}

diff --git a/src/compiler/glsl/glsl_parser_extras.h b/src/compiler/glsl/glsl_parser_extras.h
index 5aaf0bc252baaa6e4dbe2aa35c0c524ba97edb28..62f4e1fc848f05980d2a16cac81b83c3e606e479 100644 (file)
--- a/src/compiler/glsl/glsl_parser_extras.h
+++ b/src/compiler/glsl/glsl_parser_extras.h
@@ -1024,11 +1024,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);
-
 extern void
 _mesa_glsl_copy_symbols_from_table(struct exec_list *shader_ir,
                                    struct glsl_symbol_table *src,

diff --git a/src/compiler/glsl/standalone.cpp b/src/compiler/glsl/standalone.cpp
index b32fb626ef68449eb2408709ca8b974902a7a7b0..46733d490ec3834ef979f46c1c7c3a9a47e44e83 100644 (file)
--- a/src/compiler/glsl/standalone.cpp
+++ b/src/compiler/glsl/standalone.cpp
@@ -134,7 +134,7 @@ static void
 initialize_context(struct gl_context *ctx, gl_api api)
 {
    initialize_context_to_defaults(ctx, api);
-   glsl_type_singleton_init_or_ref();
+   _mesa_glsl_builtin_functions_init_or_ref();
 
    /* The standalone compiler needs to claim support for almost
     * everything in order to compile the built-in functions.
@@ -620,6 +620,5 @@ standalone_compiler_cleanup(struct gl_shader_program *whole_program)
    delete whole_program->FragDataIndexBindings;
 
    ralloc_free(whole_program);
-   glsl_type_singleton_decref();
-   _mesa_glsl_release_builtin_functions();
+   _mesa_glsl_builtin_functions_decref();
 }

diff --git a/src/mesa/main/context.c b/src/mesa/main/context.c
index 6e11f88cf1ba19d60bd9795f291c65e08be1d0fc..0128fe12d8653643c4171a73216db96bf70b0aa9 100644 (file)
--- a/src/mesa/main/context.c
+++ b/src/mesa/main/context.c
@@ -149,6 +149,7 @@
 #endif
 
 #include "compiler/glsl_types.h"
+#include "compiler/glsl/builtin_functions.h"
 #include "compiler/glsl/glsl_parser_extras.h"
 #include <stdbool.h>
 
@@ -360,7 +361,7 @@ mtx_t OneTimeLock = _MTX_INITIALIZER_NP;
 static void
 one_time_fini(void)
 {
-   _mesa_destroy_shader_compiler();
+   glsl_type_singleton_decref();
    _mesa_locale_fini();
 }
 
@@ -408,6 +409,11 @@ one_time_init( struct gl_context *ctx )
          _mesa_debug(ctx, "Mesa " PACKAGE_VERSION " DEBUG build" MESA_GIT_SHA1 "\n");
       }
 #endif
+
+      /* Take a glsl type reference for the duration of libGL's life to avoid
+       * unecessary creation/destruction of glsl types.
+       */
+      glsl_type_singleton_init_or_ref();
    }
 
    /* per-API one-time init */
@@ -1205,8 +1211,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
@@ -1394,9 +1398,6 @@ _mesa_free_context_data(struct gl_context *ctx, bool destroy_compiler_types)
 
    free(ctx->VersionString);
 
-   if (destroy_compiler_types)
-      _mesa_destroy_shader_compiler_types();
-
    ralloc_free(ctx->SoftFP64);
 
    /* unbind the context if it's currently bound */
@@ -1404,6 +1405,12 @@ _mesa_free_context_data(struct gl_context *ctx, bool destroy_compiler_types)
       _mesa_make_current(NULL, NULL, NULL);
    }
 
+   /* Do this after unbinding context to ensure any thread is finished. */
+   if (ctx->shader_builtin_ref) {
+      _mesa_glsl_builtin_functions_decref();
+      ctx->shader_builtin_ref = false;
+   }
+
    free(ctx->Const.SpirVExtensions);
 }
 

diff --git a/src/mesa/main/mtypes.h b/src/mesa/main/mtypes.h
index 569e793ca2740ad36658129219585ae755bd7d79..b38d0a290095da8a042c62e936ef2558aaf7c768 100644 (file)
--- a/src/mesa/main/mtypes.h
+++ b/src/mesa/main/mtypes.h
@@ -5174,6 +5174,8 @@ struct gl_context
    struct hash_table_u64 *ResidentTextureHandles;
    struct hash_table_u64 *ResidentImageHandles;
    /*@}*/
+
+   bool shader_builtin_ref;
 };
 
 /**

diff --git a/src/mesa/main/shaderapi.c b/src/mesa/main/shaderapi.c
index 5a0a61275125ee7d28a424b87c8ab66170eb9b42..0a9700a67612040ddc3c6da62d52db434c14b623 100644 (file)
--- a/src/mesa/main/shaderapi.c
+++ b/src/mesa/main/shaderapi.c
@@ -48,6 +48,7 @@
 #include "main/state.h"
 #include "main/transformfeedback.h"
 #include "main/uniforms.h"
+#include "compiler/glsl/builtin_functions.h"
 #include "compiler/glsl/glsl_parser_extras.h"
 #include "compiler/glsl/ir.h"
 #include "compiler/glsl/ir_uniform.h"
@@ -1155,6 +1156,14 @@ set_shader_source(struct gl_shader *sh, const GLchar *source)
 #endif
 }
 
+static void
+ensure_builtin_types(struct gl_context *ctx)
+{
+   if (!ctx->shader_builtin_ref) {
+      _mesa_glsl_builtin_functions_init_or_ref();
+      ctx->shader_builtin_ref = true;
+   }
+}
 
 /**
  * Compile a shader.
@@ -1189,6 +1198,8 @@ _mesa_compile_shader(struct gl_context *ctx, struct gl_shader *sh)
          _mesa_log("%s\n", sh->Source);
       }
 
+      ensure_builtin_types(ctx);
+
       /* this call will set the shader->CompileStatus field to indicate if
        * compilation was successful.
        */
@@ -1266,6 +1277,8 @@ link_program(struct gl_context *ctx, struct gl_shader_program *shProg,
          }
    }
 
+   ensure_builtin_types(ctx);
+
    FLUSH_VERTICES(ctx, 0);
    _mesa_glsl_link_shader(ctx, shProg);
 
@@ -2245,7 +2258,12 @@ _mesa_GetShaderPrecisionFormat(GLenum shadertype, GLenum precisiontype,
 void GLAPIENTRY
 _mesa_ReleaseShaderCompiler(void)
 {
-   _mesa_destroy_shader_compiler_caches();
+   GET_CURRENT_CONTEXT(ctx);
+
+   if (ctx->shader_builtin_ref) {
+      _mesa_glsl_builtin_functions_decref();
+      ctx->shader_builtin_ref = false;
+   }
 }
 
 

diff --git a/src/mesa/state_tracker/st_context.c b/src/mesa/state_tracker/st_context.c
index eeceaf9ddd4a2cf81e640f497c95c61f65404255..17436526c7be4b81c5f5b85e55bad58cd79dad9a 100644 (file)
--- a/src/mesa/state_tracker/st_context.c
+++ b/src/mesa/state_tracker/st_context.c
@@ -1036,11 +1036,6 @@ st_destroy_context(struct st_context *st)
    st_destroy_context_priv(st, true);
    st = NULL;
 
-   /* This must be called after st_destroy_context_priv() to avoid a race
-    * condition between any shader compiler threads and context destruction.
-    */
-   _mesa_destroy_shader_compiler_types();
-
    free(ctx);
 
    if (save_ctx == ctx) {