[Mesa-dev] [PATCH 11/20] i965: Add basic-block aware backend_instruction::insert_* methods.

[Pohjolainen, Topi]

On Thu, Jul 24, 2014 at 07:54:18PM -0700, Matt Turner wrote:
> ---
>  src/mesa/drivers/dri/i965/brw_shader.cpp | 80 ++++++++++++++++++++++++++++++++
>  src/mesa/drivers/dri/i965/brw_shader.h   |  5 ++
>  2 files changed, 85 insertions(+)
> 
> diff --git a/src/mesa/drivers/dri/i965/brw_shader.cpp b/src/mesa/drivers/dri/i965/brw_shader.cpp
> index 47535a9..ba93cbc 100644
> --- a/src/mesa/drivers/dri/i965/brw_shader.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_shader.cpp
> @@ -740,6 +740,86 @@ backend_instruction::has_side_effects() const
>  }
>  
>  void
> +backend_instruction::insert_after(bblock_t *block, backend_instruction *inst)
> +{
> +   bool found = false; (void) found;
> +   foreach_inst_in_block (backend_instruction, i, block) {
> +      if (this == i) {
> +         found = true;
> +      }
> +   }
> +   assert(found || !"Instruction not in block");
> +
> +   block->end_ip++;
> +
> +   for (bblock_t *block_iter = (bblock_t *)block->link.next;
> +        !block_iter->link.is_tail_sentinel();
> +        block_iter = (bblock_t *)block_iter->link.next) {
> +      block_iter->start_ip++;
> +      block_iter->end_ip++;
> +   }
> +
> +   if (block->end == this)
> +      block->end = inst;
> +
> +   this->insert_after(inst);

If you used "exec_node::insert_after(inst)" instead would you still need the
using directive in the header?

> +}
> +
> +void
> +backend_instruction::insert_before(bblock_t *block, backend_instruction *inst)
> +{
> +   bool found = false; (void) found;
> +   foreach_inst_in_block (backend_instruction, i, block) {
> +      if (this == i) {
> +         found = true;
> +      }
> +   }
> +   assert(found || !"Instruction not in block");
> +
> +   block->end_ip++;
> +
> +   for (bblock_t *block_iter = (bblock_t *)block->link.next;
> +        !block_iter->link.is_tail_sentinel();
> +        block_iter = (bblock_t *)block_iter->link.next) {
> +      block_iter->start_ip++;
> +      block_iter->end_ip++;
> +   }
> +
> +   if (block->start == this)
> +      block->start = inst;
> +
> +   this->insert_before(inst);
> +}
> +
> +void
> +backend_instruction::insert_before(bblock_t *block, exec_list *list)
> +{
> +   bool found = false; (void) found;
> +   foreach_inst_in_block (backend_instruction, i, block) {
> +      if (this == i) {
> +         found = true;
> +      }
> +   }
> +   assert(found || !"Instruction not in block");

This is common for all three cases, and could be refactored into its own
function, say check_inst_in_block(). It would document the seven lines nicely.

> +
> +   unsigned num_inst = list->length();
> +
> +   block->end_ip += num_inst;
> +
> +   for (bblock_t *block_iter = (bblock_t *)block->link.next;
> +        !block_iter->link.is_tail_sentinel();
> +        block_iter = (bblock_t *)block_iter->link.next) {
> +      block_iter->start_ip += num_inst;
> +      block_iter->end_ip += num_inst;
> +   }

Same here, this iteration is the same and could be its own member with
arugment telling the adjustment size.

> +
> +   if (block->start == this)
> +      block->start = (backend_instruction *)list->get_head();
> +
> +   this->insert_before(list);
> +}
> +
> +void
>  backend_instruction::remove(bblock_t *block)
>  {
>     bool found = false; (void) found;
> diff --git a/src/mesa/drivers/dri/i965/brw_shader.h b/src/mesa/drivers/dri/i965/brw_shader.h
> index 4b80ea9..d174d5c 100644
> --- a/src/mesa/drivers/dri/i965/brw_shader.h
> +++ b/src/mesa/drivers/dri/i965/brw_shader.h
> @@ -92,6 +92,11 @@ struct backend_instruction : public exec_node {
>  
>     using exec_node::remove;
>     void remove(bblock_t *block);
> +   using exec_node::insert_after;
> +   void insert_after(bblock_t *block, backend_instruction *inst);
> +   using exec_node::insert_before;
> +   void insert_before(bblock_t *block, backend_instruction *inst);
> +   void insert_before(bblock_t *block, exec_list *list);
>  
>     /**
>      * True if the instruction has side effects other than writing to
> -- 
> 1.8.5.5
> 
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev