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

[Kenneth Graunke]

On Tuesday, August 8, 2017 8:34:07 PM PDT Timothy Arceri wrote:
> 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%.
> 
> Cc: Kenneth Graunke 
> Cc: Lionel Landwerlin 
> 
> Tested-by: Dieter Nützel 

With 5v2, this is:

Reviewed-by: Kenneth Graunke 

Thanks!


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

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

[Timothy Arceri]

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

Cc: Kenneth Graunke 
Cc: Lionel Landwerlin 

Tested-by: Dieter Nützel 
---
 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 a63f545b79..9df9671f13 100644
--- a/src/compiler/glsl/builtin_functions.cpp
+++ b/src/compiler/glsl/builtin_functions.cpp
@@ -6284,30 +6284,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.4

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

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

[Timothy Arceri]

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

Cc: Kenneth Graunke 
Cc: Lionel Landwerlin 

Tested-by: Dieter Nützel 
---
 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@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev