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@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
