Re: [Mesa-dev] [PATCH 2/4] i965: Add a copy constructor (of sorts) for cfg_t.

2018-02-23 Thread Francisco Jerez
Kenneth Graunke  writes:

> This clones an existing CFG.
>
> We can't properly copy the instruction lists from cfg_t itself, as
> it's fs_inst/vec4_instruction agnostic, and we don't use templates.
> To accomplish this, we pass in an instruction-list-copying function.
> ---
>  src/intel/compiler/brw_cfg.cpp | 41 +
>  src/intel/compiler/brw_cfg.h   |  6 ++
>  2 files changed, 47 insertions(+)
>
> diff --git a/src/intel/compiler/brw_cfg.cpp b/src/intel/compiler/brw_cfg.cpp
> index 600b428a492..06cb6324a7e 100644
> --- a/src/intel/compiler/brw_cfg.cpp
> +++ b/src/intel/compiler/brw_cfg.cpp
> @@ -386,6 +386,47 @@ cfg_t::cfg_t(exec_list *instructions)
> make_block_array();
>  }
>  
> +cfg_t::cfg_t(const cfg_t ,
> + void (*copy_instruction_list)(void *mem_ctx, exec_list *dst,
> +   exec_list *src))

I think it would be cleaner and more robust to use dynamic dispatch to
select the right copy function for this.  E.g. by defining a "virtual
backend_instruction *clone() const" method in the backend_instruction
class which is equivalent to calling the copy constructor with *this as
argument on a new object.  That way you'd get a real copy constructor.
Also by doing that you wouldn't need to re-implement the exec_list
traversal for each instruction type you want to copy, it can be done in
the loop below.

> +{
> +   mem_ctx = ralloc_context(NULL);
> +   idom_dirty = other.idom_dirty;
> +   num_blocks = other.num_blocks;
> +   cycle_count = other.cycle_count;
> +
> +   foreach_block(other_block, ) {
> +  bblock_t *block = new(mem_ctx) bblock_t(this);
> +
> +  block->start_ip = other_block->start_ip;
> +  block->end_ip = other_block->end_ip;
> +  block->num = other_block->num;
> +  block->cycle_count = other_block->cycle_count;
> +
> +  block_list.push_tail(>link);
> +   }
> +
> +   make_block_array();
> +
> +   for (int b = 0; b < num_blocks; b++) {
> +  copy_instruction_list(mem_ctx,
> +[b]->instructions,
> +[b]->instructions);
> +  foreach_list_typed(bblock_link, predecessor, link,
> + [b]->parents) {
> + bblock_link *new_link =
> +new(mem_ctx) bblock_link(blocks[predecessor->block->num]);
> + blocks[b]->parents.push_tail(_link->link);
> +  }
> +  foreach_list_typed(bblock_link, successor, link,
> + [b]->children) {
> + bblock_link *new_link =
> +new(mem_ctx) bblock_link(blocks[successor->block->num]);
> + blocks[b]->children.push_tail(_link->link);
> +  }
> +   }
> +}
> +
>  cfg_t::~cfg_t()
>  {
> ralloc_free(mem_ctx);
> diff --git a/src/intel/compiler/brw_cfg.h b/src/intel/compiler/brw_cfg.h
> index 0c5d1268f1a..7d9f4ba9622 100644
> --- a/src/intel/compiler/brw_cfg.h
> +++ b/src/intel/compiler/brw_cfg.h
> @@ -273,9 +273,15 @@ bblock_t::last_non_control_flow_inst()
>  
>  struct cfg_t {
>  #ifdef __cplusplus
> +private:
> +   cfg_t(const cfg_t );
> +
> +public:
> DECLARE_RALLOC_CXX_OPERATORS(cfg_t)
>  
> cfg_t(exec_list *instructions);
> +   cfg_t(const cfg_t ,
> + void (*copy_instlist)(void *mem_ctx, exec_list *dst, exec_list 
> *src));
> ~cfg_t();
>  
> void remove_block(bblock_t *block);
> -- 
> 2.16.1
>
> ___
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev


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


[Mesa-dev] [PATCH 2/4] i965: Add a copy constructor (of sorts) for cfg_t.

2018-02-23 Thread Kenneth Graunke
This clones an existing CFG.

We can't properly copy the instruction lists from cfg_t itself, as
it's fs_inst/vec4_instruction agnostic, and we don't use templates.
To accomplish this, we pass in an instruction-list-copying function.
---
 src/intel/compiler/brw_cfg.cpp | 41 +
 src/intel/compiler/brw_cfg.h   |  6 ++
 2 files changed, 47 insertions(+)

diff --git a/src/intel/compiler/brw_cfg.cpp b/src/intel/compiler/brw_cfg.cpp
index 600b428a492..06cb6324a7e 100644
--- a/src/intel/compiler/brw_cfg.cpp
+++ b/src/intel/compiler/brw_cfg.cpp
@@ -386,6 +386,47 @@ cfg_t::cfg_t(exec_list *instructions)
make_block_array();
 }
 
+cfg_t::cfg_t(const cfg_t ,
+ void (*copy_instruction_list)(void *mem_ctx, exec_list *dst,
+   exec_list *src))
+{
+   mem_ctx = ralloc_context(NULL);
+   idom_dirty = other.idom_dirty;
+   num_blocks = other.num_blocks;
+   cycle_count = other.cycle_count;
+
+   foreach_block(other_block, ) {
+  bblock_t *block = new(mem_ctx) bblock_t(this);
+
+  block->start_ip = other_block->start_ip;
+  block->end_ip = other_block->end_ip;
+  block->num = other_block->num;
+  block->cycle_count = other_block->cycle_count;
+
+  block_list.push_tail(>link);
+   }
+
+   make_block_array();
+
+   for (int b = 0; b < num_blocks; b++) {
+  copy_instruction_list(mem_ctx,
+[b]->instructions,
+[b]->instructions);
+  foreach_list_typed(bblock_link, predecessor, link,
+ [b]->parents) {
+ bblock_link *new_link =
+new(mem_ctx) bblock_link(blocks[predecessor->block->num]);
+ blocks[b]->parents.push_tail(_link->link);
+  }
+  foreach_list_typed(bblock_link, successor, link,
+ [b]->children) {
+ bblock_link *new_link =
+new(mem_ctx) bblock_link(blocks[successor->block->num]);
+ blocks[b]->children.push_tail(_link->link);
+  }
+   }
+}
+
 cfg_t::~cfg_t()
 {
ralloc_free(mem_ctx);
diff --git a/src/intel/compiler/brw_cfg.h b/src/intel/compiler/brw_cfg.h
index 0c5d1268f1a..7d9f4ba9622 100644
--- a/src/intel/compiler/brw_cfg.h
+++ b/src/intel/compiler/brw_cfg.h
@@ -273,9 +273,15 @@ bblock_t::last_non_control_flow_inst()
 
 struct cfg_t {
 #ifdef __cplusplus
+private:
+   cfg_t(const cfg_t );
+
+public:
DECLARE_RALLOC_CXX_OPERATORS(cfg_t)
 
cfg_t(exec_list *instructions);
+   cfg_t(const cfg_t ,
+ void (*copy_instlist)(void *mem_ctx, exec_list *dst, exec_list *src));
~cfg_t();
 
void remove_block(bblock_t *block);
-- 
2.16.1

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