[Mesa-dev] [PATCH 3/3] glsl: stop cloning builtin fuctions _mesa_glsl_find_builtin_function()

[Thomas Helland]

2017-08-04 7:25 GMT+00:00 Timothy Arceri <tarceri at itsqueeze.com>:
> The cloning was introduced in f81ede469910d to fixed a problem with
> shaders including IR that was owned by builtins.
>
> However the approach of cloning the whole function each time we
> reference a builtin lead to a significant reduction in the GLSL
> IR compilers performance.
>
> The previous patch fixes the ownership problem in a more precise
> way. So we can now remove this cloning.
>
> Testing on a Ryzan 7 1800X shows a ~15% decreases in compiling the
> Deus Ex: Mankind Divided shaders on radeonsi (which take 5min+ on
> some machines). Looking just at the GLSL IR compiler the speed up
> is ~40%.
>

While I haven't looked at patch 2 closely enough to give you an RB,
these performance numbers seem quite in line with what I observed
when profiling this back in June. Nice one =)

> Cc: Kenneth Graunke <kenneth at whitecape.org>
> Cc: Lionel Landwerlin <lionel.g.landwerlin at intel.com>
> ---
>  src/compiler/glsl/builtin_functions.cpp | 11 +----------
>  1 file changed, 1 insertion(+), 10 deletions(-)
>
> diff --git a/src/compiler/glsl/builtin_functions.cpp b/src/compiler/glsl/builtin_functions.cpp
> index 84833bdd7d..1393087cc6 100644
> --- a/src/compiler/glsl/builtin_functions.cpp
> +++ b/src/compiler/glsl/builtin_functions.cpp
> @@ -6207,30 +6207,21 @@ _mesa_glsl_release_builtin_functions()
>
>  ir_function_signature *
>  _mesa_glsl_find_builtin_function(_mesa_glsl_parse_state *state,
>                                   const char *name, exec_list *actual_parameters)
>  {
>     ir_function_signature *s;
>     mtx_lock(&builtins_lock);
>     s = builtins.find(state, name, actual_parameters);
>     mtx_unlock(&builtins_lock);
>
> -   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);
> +   return s;
>  }
>
>  bool
>  _mesa_glsl_has_builtin_function(_mesa_glsl_parse_state *state, const char *name)
>  {
>     ir_function *f;
>     bool ret = false;
>     mtx_lock(&builtins_lock);
>     f = builtins.shader->symbols->get_function(name);
>     if (f != NULL) {
> --
> 2.13.3
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev