[Mesa-dev] [PATCH 2/2] glsl: Use hash table cloning in copy propagation

2018-03-13 Thread Thomas Helland 
Walking the whole hash table, inserting entries by hashing them first
is just a really bad idea. We can simply memcpy the whole thing.

While this does not have a major performance impact on average,
as it only helps shaders with a lot of branches, it might help
individual shaders quite a lot. For my shader-db I get a reduction
from 1'381 (+-0,03%) to 1'272 (+-0,03%) billion cycles on five runs,
as reported by "perf stat".

V2: Remove leftover creation of acp in two places

Reviewed-by: Eric Anholt 
---
 src/compiler/glsl/opt_copy_propagation.cpp | 17 -
 .../glsl/opt_copy_propagation_elements.cpp | 29 --
 2 files changed, 15 insertions(+), 31 deletions(-)

diff --git a/src/compiler/glsl/opt_copy_propagation.cpp 
b/src/compiler/glsl/opt_copy_propagation.cpp
index e904e6ede4..6220aa86da 100644
--- a/src/compiler/glsl/opt_copy_propagation.cpp
+++ b/src/compiler/glsl/opt_copy_propagation.cpp
@@ -213,17 +213,12 @@ ir_copy_propagation_visitor::handle_if_block(exec_list 
*instructions)
set *orig_kills = this->kills;
bool orig_killed_all = this->killed_all;
 
-   acp = _mesa_hash_table_create(NULL, _mesa_hash_pointer,
- _mesa_key_pointer_equal);
kills = _mesa_set_create(NULL, _mesa_hash_pointer,
 _mesa_key_pointer_equal);
this->killed_all = false;
 
/* Populate the initial acp with a copy of the original */
-   struct hash_entry *entry;
-   hash_table_foreach(orig_acp, entry) {
-  _mesa_hash_table_insert(acp, entry->key, entry->data);
-   }
+   acp = _mesa_hash_table_clone(orig_acp, NULL);
 
visit_list_elements(this, instructions);
 
@@ -264,17 +259,15 @@ ir_copy_propagation_visitor::handle_loop(ir_loop *ir, 
bool keep_acp)
set *orig_kills = this->kills;
bool orig_killed_all = this->killed_all;
 
-   acp = _mesa_hash_table_create(NULL, _mesa_hash_pointer,
- _mesa_key_pointer_equal);
kills = _mesa_set_create(NULL, _mesa_hash_pointer,
 _mesa_key_pointer_equal);
this->killed_all = false;
 
if (keep_acp) {
-  struct hash_entry *entry;
-  hash_table_foreach(orig_acp, entry) {
- _mesa_hash_table_insert(acp, entry->key, entry->data);
-  }
+  acp = _mesa_hash_table_clone(orig_acp, NULL);
+   } else {
+  acp = _mesa_hash_table_create(NULL, _mesa_hash_pointer,
+_mesa_key_pointer_equal);
}
 
visit_list_elements(this, >body_instructions);
diff --git a/src/compiler/glsl/opt_copy_propagation_elements.cpp 
b/src/compiler/glsl/opt_copy_propagation_elements.cpp
index 9f79fa9202..8bae424a1d 100644
--- a/src/compiler/glsl/opt_copy_propagation_elements.cpp
+++ b/src/compiler/glsl/opt_copy_propagation_elements.cpp
@@ -124,6 +124,12 @@ public:
   ralloc_free(mem_ctx);
}
 
+   void clone_acp(hash_table *lhs, hash_table *rhs)
+   {
+  lhs_ht = _mesa_hash_table_clone(lhs, mem_ctx);
+  rhs_ht = _mesa_hash_table_clone(rhs, mem_ctx);
+   }
+
void create_acp()
{
   lhs_ht = _mesa_hash_table_create(mem_ctx, _mesa_hash_pointer,
@@ -138,19 +144,6 @@ public:
   _mesa_hash_table_destroy(rhs_ht, NULL);
}
 
-   void populate_acp(hash_table *lhs, hash_table *rhs)
-   {
-  struct hash_entry *entry;
-
-  hash_table_foreach(lhs, entry) {
- _mesa_hash_table_insert(lhs_ht, entry->key, entry->data);
-  }
-
-  hash_table_foreach(rhs, entry) {
- _mesa_hash_table_insert(rhs_ht, entry->key, entry->data);
-  }
-   }
-
void handle_loop(ir_loop *, bool keep_acp);
virtual ir_visitor_status visit_enter(class ir_loop *);
virtual ir_visitor_status visit_enter(class ir_function_signature *);
@@ -395,10 +388,8 @@ 
ir_copy_propagation_elements_visitor::handle_if_block(exec_list *instructions)
this->kills = new(mem_ctx) exec_list;
this->killed_all = false;
 
-   create_acp();
-
/* Populate the initial acp with a copy of the original */
-   populate_acp(orig_lhs_ht, orig_rhs_ht);
+   clone_acp(orig_lhs_ht, orig_rhs_ht);
 
visit_list_elements(this, instructions);
 
@@ -454,11 +445,11 @@ ir_copy_propagation_elements_visitor::handle_loop(ir_loop 
*ir, bool keep_acp)
this->kills = new(mem_ctx) exec_list;
this->killed_all = false;
 
-   create_acp();
-
if (keep_acp) {
   /* Populate the initial acp with a copy of the original */
-  populate_acp(orig_lhs_ht, orig_rhs_ht);
+  clone_acp(orig_lhs_ht, orig_rhs_ht);
+   } else {
+  create_acp();
}
 
visit_list_elements(this, >body_instructions);
-- 
2.16.2

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

Re: [Mesa-dev] [PATCH 2/2] glsl: Use hash table cloning in copy propagation

2018-03-12 Thread Eric Anholt 
Thomas Helland  writes:

> Walking the whole hash table, inserting entries by hashing them first
> is just a really bad idea. We can simply memcpy the whole thing.
> ---
>  src/compiler/glsl/opt_copy_propagation.cpp | 13 --
>  .../glsl/opt_copy_propagation_elements.cpp | 29 
> --
>  2 files changed, 15 insertions(+), 27 deletions(-)
>
> diff --git a/src/compiler/glsl/opt_copy_propagation.cpp 
> b/src/compiler/glsl/opt_copy_propagation.cpp
> index e904e6ede4..96667779da 100644
> --- a/src/compiler/glsl/opt_copy_propagation.cpp
> +++ b/src/compiler/glsl/opt_copy_propagation.cpp
> @@ -220,10 +220,7 @@ ir_copy_propagation_visitor::handle_if_block(exec_list 
> *instructions)
> this->killed_all = false;
>  
> /* Populate the initial acp with a copy of the original */
> -   struct hash_entry *entry;
> -   hash_table_foreach(orig_acp, entry) {
> -  _mesa_hash_table_insert(acp, entry->key, entry->data);
> -   }
> +   acp = _mesa_hash_table_clone(orig_acp, NULL);

Remove creation of acp above

>  
> visit_list_elements(this, instructions);
>  
> @@ -271,10 +268,10 @@ ir_copy_propagation_visitor::handle_loop(ir_loop *ir, 
> bool keep_acp)
> this->killed_all = false;
>  
> if (keep_acp) {
> -  struct hash_entry *entry;
> -  hash_table_foreach(orig_acp, entry) {
> - _mesa_hash_table_insert(acp, entry->key, entry->data);
> -  }
> +  acp = _mesa_hash_table_clone(orig_acp, NULL);
> +   } else {
> +  acp = _mesa_hash_table_create(NULL, _mesa_hash_pointer,
> +_mesa_key_pointer_equal);
> }

Again, remove the old creation of the acp.

Other than that, these are:

Reviewed-by: Eric Anholt 


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

Re: [Mesa-dev] [PATCH 2/2] glsl: Use hash table cloning in copy propagation

2018-03-12 Thread Emil Velikov 
Hi Thomas,

If I were you I'd split out the introduction of clone_acp() into a
separate patch.
Regardless of that suggestions, there seems to be a bug in this patch.

On 12 March 2018 at 17:55, Thomas Helland  wrote:
> Walking the whole hash table, inserting entries by hashing them first
> is just a really bad idea. We can simply memcpy the whole thing.
> ---
>  src/compiler/glsl/opt_copy_propagation.cpp | 13 --
>  .../glsl/opt_copy_propagation_elements.cpp | 29 
> --
>  2 files changed, 15 insertions(+), 27 deletions(-)
>
> diff --git a/src/compiler/glsl/opt_copy_propagation.cpp 
> b/src/compiler/glsl/opt_copy_propagation.cpp
> index e904e6ede4..96667779da 100644
> --- a/src/compiler/glsl/opt_copy_propagation.cpp
> +++ b/src/compiler/glsl/opt_copy_propagation.cpp
> @@ -220,10 +220,7 @@ ir_copy_propagation_visitor::handle_if_block(exec_list 
> *instructions)
> this->killed_all = false;
>
> /* Populate the initial acp with a copy of the original */
> -   struct hash_entry *entry;
> -   hash_table_foreach(orig_acp, entry) {
> -  _mesa_hash_table_insert(acp, entry->key, entry->data);
> -   }
> +   acp = _mesa_hash_table_clone(orig_acp, NULL);
>
There's a _mesa_hash_table_create just above that should be removed.

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

[Mesa-dev] [PATCH 2/2] glsl: Use hash table cloning in copy propagation

2018-03-12 Thread Thomas Helland 
Walking the whole hash table, inserting entries by hashing them first
is just a really bad idea. We can simply memcpy the whole thing.
---
 src/compiler/glsl/opt_copy_propagation.cpp | 13 --
 .../glsl/opt_copy_propagation_elements.cpp | 29 --
 2 files changed, 15 insertions(+), 27 deletions(-)

diff --git a/src/compiler/glsl/opt_copy_propagation.cpp 
b/src/compiler/glsl/opt_copy_propagation.cpp
index e904e6ede4..96667779da 100644
--- a/src/compiler/glsl/opt_copy_propagation.cpp
+++ b/src/compiler/glsl/opt_copy_propagation.cpp
@@ -220,10 +220,7 @@ ir_copy_propagation_visitor::handle_if_block(exec_list 
*instructions)
this->killed_all = false;
 
/* Populate the initial acp with a copy of the original */
-   struct hash_entry *entry;
-   hash_table_foreach(orig_acp, entry) {
-  _mesa_hash_table_insert(acp, entry->key, entry->data);
-   }
+   acp = _mesa_hash_table_clone(orig_acp, NULL);
 
visit_list_elements(this, instructions);
 
@@ -271,10 +268,10 @@ ir_copy_propagation_visitor::handle_loop(ir_loop *ir, 
bool keep_acp)
this->killed_all = false;
 
if (keep_acp) {
-  struct hash_entry *entry;
-  hash_table_foreach(orig_acp, entry) {
- _mesa_hash_table_insert(acp, entry->key, entry->data);
-  }
+  acp = _mesa_hash_table_clone(orig_acp, NULL);
+   } else {
+  acp = _mesa_hash_table_create(NULL, _mesa_hash_pointer,
+_mesa_key_pointer_equal);
}
 
visit_list_elements(this, >body_instructions);
diff --git a/src/compiler/glsl/opt_copy_propagation_elements.cpp 
b/src/compiler/glsl/opt_copy_propagation_elements.cpp
index 9f79fa9202..8bae424a1d 100644
--- a/src/compiler/glsl/opt_copy_propagation_elements.cpp
+++ b/src/compiler/glsl/opt_copy_propagation_elements.cpp
@@ -124,6 +124,12 @@ public:
   ralloc_free(mem_ctx);
}
 
+   void clone_acp(hash_table *lhs, hash_table *rhs)
+   {
+  lhs_ht = _mesa_hash_table_clone(lhs, mem_ctx);
+  rhs_ht = _mesa_hash_table_clone(rhs, mem_ctx);
+   }
+
void create_acp()
{
   lhs_ht = _mesa_hash_table_create(mem_ctx, _mesa_hash_pointer,
@@ -138,19 +144,6 @@ public:
   _mesa_hash_table_destroy(rhs_ht, NULL);
}
 
-   void populate_acp(hash_table *lhs, hash_table *rhs)
-   {
-  struct hash_entry *entry;
-
-  hash_table_foreach(lhs, entry) {
- _mesa_hash_table_insert(lhs_ht, entry->key, entry->data);
-  }
-
-  hash_table_foreach(rhs, entry) {
- _mesa_hash_table_insert(rhs_ht, entry->key, entry->data);
-  }
-   }
-
void handle_loop(ir_loop *, bool keep_acp);
virtual ir_visitor_status visit_enter(class ir_loop *);
virtual ir_visitor_status visit_enter(class ir_function_signature *);
@@ -395,10 +388,8 @@ 
ir_copy_propagation_elements_visitor::handle_if_block(exec_list *instructions)
this->kills = new(mem_ctx) exec_list;
this->killed_all = false;
 
-   create_acp();
-
/* Populate the initial acp with a copy of the original */
-   populate_acp(orig_lhs_ht, orig_rhs_ht);
+   clone_acp(orig_lhs_ht, orig_rhs_ht);
 
visit_list_elements(this, instructions);
 
@@ -454,11 +445,11 @@ ir_copy_propagation_elements_visitor::handle_loop(ir_loop 
*ir, bool keep_acp)
this->kills = new(mem_ctx) exec_list;
this->killed_all = false;
 
-   create_acp();
-
if (keep_acp) {
   /* Populate the initial acp with a copy of the original */
-  populate_acp(orig_lhs_ht, orig_rhs_ht);
+  clone_acp(orig_lhs_ht, orig_rhs_ht);
+   } else {
+  create_acp();
}
 
visit_list_elements(this, >body_instructions);
-- 
2.15.1

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