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

[Nicolai Hähnle]

On 04.08.2017 09:25, Timothy Arceri wrote:
> The cloning was introduced in f81ede469910d to fixed a problem with

*to fix

> 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

*Ryzen

Okay, generate_call immediately inlines builtin functions. So the normal 
case should not cross memory contexts, and the previous patch fixes the 
case of constant expression evaluation. Seems reasonable.

Assuming you fix the minor comments above and the comment in patch #2, 
the series is:

Reviewed-by: Nicolai Hähnle <nicolai.haehnle at amd.com>


> 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%.
> 
> 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) {
> 


-- 
Lerne, wie die Welt wirklich ist,
Aber vergiss niemals, wie sie sein sollte.