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

[Francisco Jerez]

Kenneth Graunke <kenneth at whitecape.org> 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 &other,
> +             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, &other) {
> +      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(&block->link);
> +   }
> +
> +   make_block_array();
> +
> +   for (int b = 0; b < num_blocks; b++) {
> +      copy_instruction_list(mem_ctx,
> +                            &blocks[b]->instructions,
> +                            &other.blocks[b]->instructions);
> +      foreach_list_typed(bblock_link, predecessor, link,
> +                         &other.blocks[b]->parents) {
> +         bblock_link *new_link =
> +            new(mem_ctx) bblock_link(blocks[predecessor->block->num]);
> +         blocks[b]->parents.push_tail(&new_link->link);
> +      }
> +      foreach_list_typed(bblock_link, successor, link,
> +                         &other.blocks[b]->children) {
> +         bblock_link *new_link =
> +            new(mem_ctx) bblock_link(blocks[successor->block->num]);
> +         blocks[b]->children.push_tail(&new_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 &other);
> +
> +public:
>     DECLARE_RALLOC_CXX_OPERATORS(cfg_t)
>  
>     cfg_t(exec_list *instructions);
> +   cfg_t(const cfg_t &other,
> +         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 at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 227 bytes
Desc: not available
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20180223/282a05ab/attachment.sig>