On Tue, Sep 02, 2014 at 09:34:28PM -0700, Matt Turner wrote: > --- > src/mesa/drivers/dri/i965/brw_cfg.cpp | 62 ++++++++--------- > src/mesa/drivers/dri/i965/brw_cfg.h | 77 > +++++++++++++++++----- > .../drivers/dri/i965/brw_dead_control_flow.cpp | 6 +- > src/mesa/drivers/dri/i965/brw_fs.cpp | 6 +- > src/mesa/drivers/dri/i965/brw_fs_cse.cpp | 6 -- > .../dri/i965/brw_fs_peephole_predicated_break.cpp | 6 +- > src/mesa/drivers/dri/i965/brw_fs_reg_allocate.cpp | 4 +- > src/mesa/drivers/dri/i965/brw_fs_sel_peephole.cpp | 4 +- > .../drivers/dri/i965/brw_schedule_instructions.cpp | 10 +-- > src/mesa/drivers/dri/i965/brw_shader.cpp | 14 ---- > src/mesa/drivers/dri/i965/brw_vec4_cse.cpp | 6 -- > src/mesa/drivers/dri/i965/intel_asm_annotation.c | 4 +- > 12 files changed, 114 insertions(+), 91 deletions(-) > > diff --git a/src/mesa/drivers/dri/i965/brw_cfg.cpp > b/src/mesa/drivers/dri/i965/brw_cfg.cpp > index 8714b68..44e7744 100644 > --- a/src/mesa/drivers/dri/i965/brw_cfg.cpp > +++ b/src/mesa/drivers/dri/i965/brw_cfg.cpp
I really like this. Indentation in this file looks weird but it is the current code using tabs and your changes spaces. Reviewed-by: Topi Pohjolainen <topi.pohjolai...@intel.com> > @@ -54,9 +54,7 @@ bblock_t::bblock_t(cfg_t *cfg) : > cfg(cfg), start_ip(0), end_ip(0), num(0), > if_block(NULL), else_block(NULL) > { > - start = NULL; > - end = NULL; > - > + instructions.make_empty(); > parents.make_empty(); > children.make_empty(); > } > @@ -119,8 +117,8 @@ bblock_t::can_combine_with(const bblock_t *that) const > if ((const bblock_t *)this->link.next != that) > return false; > > - if (ends_block(this->end) || > - starts_block(that->start)) > + if (ends_block(this->end()) || > + starts_block(that->start())) > return false; > > return true; > @@ -138,8 +136,8 @@ bblock_t::combine_with(bblock_t *that) > } > > this->end_ip = that->end_ip; > - this->end = that->end; > this->else_block = that->else_block; > + this->instructions.append_list(&that->instructions); > > this->cfg->remove_block(that); > } > @@ -148,9 +146,7 @@ void > bblock_t::dump(backend_visitor *v) > { > int ip = this->start_ip; > - for (backend_instruction *inst = (backend_instruction *)this->start; > - inst != this->end->next; > - inst = (backend_instruction *) inst->next) { > + foreach_inst_in_block(backend_instruction, inst, this) { > fprintf(stderr, "%5d: ", ip); > v->dump_instruction(inst); > ip++; > @@ -178,16 +174,15 @@ cfg_t::cfg_t(exec_list *instructions) > > set_next_block(&cur, entry, ip); > > - entry->start = (backend_instruction *) instructions->get_head(); > - > - foreach_in_list(backend_instruction, inst, instructions) { > - cur->end = inst; > - > + foreach_in_list_safe(backend_instruction, inst, instructions) { > /* set_next_block wants the post-incremented ip */ > ip++; > > switch (inst->opcode) { > case BRW_OPCODE_IF: > + inst->remove(); > + cur->instructions.push_tail(inst); > + > /* Push our information onto a stack so we can recover from > * nested ifs. > */ > @@ -202,44 +197,46 @@ cfg_t::cfg_t(exec_list *instructions) > * instructions. > */ > next = new_block(); > - next->start = (backend_instruction *)inst->next; > cur_if->add_successor(mem_ctx, next); > > set_next_block(&cur, next, ip); > break; > > case BRW_OPCODE_ELSE: > + inst->remove(); > + cur->instructions.push_tail(inst); > + > cur_else = cur; > > next = new_block(); > - next->start = (backend_instruction *)inst->next; > cur_if->add_successor(mem_ctx, next); > > set_next_block(&cur, next, ip); > break; > > case BRW_OPCODE_ENDIF: { > - if (cur->start == inst) { > + if (cur->instructions.is_empty()) { > /* New block was just created; use it. */ > cur_endif = cur; > } else { > cur_endif = new_block(); > - cur_endif->start = inst; > > - cur->end = (backend_instruction *)inst->prev; > cur->add_successor(mem_ctx, cur_endif); > > set_next_block(&cur, cur_endif, ip - 1); > } > > + inst->remove(); > + cur->instructions.push_tail(inst); > + > if (cur_else) { > cur_else->add_successor(mem_ctx, cur_endif); > } else { > cur_if->add_successor(mem_ctx, cur_endif); > } > > - assert(cur_if->end->opcode == BRW_OPCODE_IF); > - assert(!cur_else || cur_else->end->opcode == BRW_OPCODE_ELSE); > + assert(cur_if->end()->opcode == BRW_OPCODE_IF); > + assert(!cur_else || cur_else->end()->opcode == BRW_OPCODE_ELSE); > > cur_if->if_block = cur_if; > cur_if->else_block = cur_else; > @@ -269,25 +266,28 @@ cfg_t::cfg_t(exec_list *instructions) > */ > cur_while = new_block(); > > - if (cur->start == inst) { > + if (cur->instructions.is_empty()) { > /* New block was just created; use it. */ > cur_do = cur; > } else { > cur_do = new_block(); > - cur_do->start = inst; > > - cur->end = (backend_instruction *)inst->prev; > cur->add_successor(mem_ctx, cur_do); > > set_next_block(&cur, cur_do, ip - 1); > } > + > + inst->remove(); > + cur->instructions.push_tail(inst); > break; > > case BRW_OPCODE_CONTINUE: > + inst->remove(); > + cur->instructions.push_tail(inst); > + > cur->add_successor(mem_ctx, cur_do); > > next = new_block(); > - next->start = (backend_instruction *)inst->next; > if (inst->predicate) > cur->add_successor(mem_ctx, next); > > @@ -295,10 +295,12 @@ cfg_t::cfg_t(exec_list *instructions) > break; > > case BRW_OPCODE_BREAK: > + inst->remove(); > + cur->instructions.push_tail(inst); > + > cur->add_successor(mem_ctx, cur_while); > > next = new_block(); > - next->start = (backend_instruction *)inst->next; > if (inst->predicate) > cur->add_successor(mem_ctx, next); > > @@ -306,7 +308,8 @@ cfg_t::cfg_t(exec_list *instructions) > break; > > case BRW_OPCODE_WHILE: > - cur_while->start = (backend_instruction *)inst->next; > + inst->remove(); > + cur->instructions.push_tail(inst); > > cur->add_successor(mem_ctx, cur_do); > set_next_block(&cur, cur_while, ip); > @@ -317,12 +320,12 @@ cfg_t::cfg_t(exec_list *instructions) > break; > > default: > + inst->remove(); > + cur->instructions.push_tail(inst); > break; > } > } > > - assert(cur->end); > - > cur->end_ip = ip; > > make_block_array(); > @@ -397,7 +400,6 @@ void > cfg_t::set_next_block(bblock_t **cur, bblock_t *block, int ip) > { > if (*cur) { > - assert((*cur)->end->next == block->start); > (*cur)->end_ip = ip - 1; > } > > diff --git a/src/mesa/drivers/dri/i965/brw_cfg.h > b/src/mesa/drivers/dri/i965/brw_cfg.h > index 94713df..3511914 100644 > --- a/src/mesa/drivers/dri/i965/brw_cfg.h > +++ b/src/mesa/drivers/dri/i965/brw_cfg.h > @@ -47,9 +47,7 @@ struct bblock_link { > struct bblock_t *block; > }; > > -#ifndef __cplusplus > struct backend_instruction; > -#endif > > struct bblock_t { > #ifdef __cplusplus > @@ -63,17 +61,20 @@ struct bblock_t { > bool can_combine_with(const bblock_t *that) const; > void combine_with(bblock_t *that); > void dump(backend_visitor *v); > + > + backend_instruction *start(); > + const backend_instruction *start() const; > + backend_instruction *end(); > + const backend_instruction *end() const; > #endif > > struct exec_node link; > struct cfg_t *cfg; > > - struct backend_instruction *start; > - struct backend_instruction *end; > - > int start_ip; > int end_ip; > > + struct exec_list instructions; > struct exec_list parents; > struct exec_list children; > int num; > @@ -87,6 +88,56 @@ struct bblock_t { > struct bblock_t *else_block; > }; > > +static inline struct backend_instruction * > +bblock_start(struct bblock_t *block) > +{ > + return (struct backend_instruction > *)exec_list_get_head(&block->instructions); > +} > + > +static inline const struct backend_instruction * > +bblock_start_const(const struct bblock_t *block) > +{ > + return (const struct backend_instruction > *)exec_list_get_head_const(&block->instructions); > +} > + > +static inline struct backend_instruction * > +bblock_end(struct bblock_t *block) > +{ > + return (struct backend_instruction > *)exec_list_get_tail(&block->instructions); > +} > + > +static inline const struct backend_instruction * > +bblock_end_const(const struct bblock_t *block) > +{ > + return (const struct backend_instruction > *)exec_list_get_tail_const(&block->instructions); > +} > + > +#ifdef __cplusplus > +inline backend_instruction * > +bblock_t::start() > +{ > + return bblock_start(this); > +} > + > +inline const backend_instruction * > +bblock_t::start() const > +{ > + return bblock_start_const(this); > +} > + > +inline backend_instruction * > +bblock_t::end() > +{ > + return bblock_end(this); > +} > + > +inline const backend_instruction * > +bblock_t::end() const > +{ > + return bblock_end_const(this); > +} > +#endif > + > struct cfg_t { > #ifdef __cplusplus > DECLARE_RALLOC_CXX_OPERATORS(cfg_t) > @@ -131,31 +182,27 @@ struct cfg_t { > foreach_list_typed_safe (bblock_t, __block, link, &(__cfg)->block_list) > > #define foreach_inst_in_block(__type, __inst, __block) \ > - for (__type *__inst = (__type *)__block->start; \ > - __inst != __block->end->next; \ > - __inst = (__type *)__inst->next) > + foreach_in_list(__type, __inst, &(__block)->instructions) > > #define foreach_inst_in_block_safe(__type, __inst, __block) \ > - for (__type *__inst = (__type *)__block->start, \ > + for (__type *__inst = (__type *)__block->instructions.head, \ > *__next = (__type *)__inst->next, \ > - *__end = (__type *)__block->end->next->next; \ > + *__end = (__type *)__block->instructions.tail; \ > __next != __end; \ > __inst = __next, \ > __next = (__type *)__next->next) > > #define foreach_inst_in_block_reverse(__type, __inst, __block) \ > - for (__type *__inst = (__type *)__block->end; \ > - __inst != __block->start->prev; \ > - __inst = (__type *)__inst->prev) > + foreach_in_list_reverse(__type, __inst, &(__block)->instructions) > > #define foreach_inst_in_block_starting_from(__type, __scan_inst, __inst, > __block) \ > for (__type *__scan_inst = (__type *)__inst->next; \ > - __scan_inst != __block->end->next; \ > + !__scan_inst->is_tail_sentinel(); \ > __scan_inst = (__type *)__scan_inst->next) > > #define foreach_inst_in_block_reverse_starting_from(__type, __scan_inst, > __inst, __block) \ > for (__type *__scan_inst = (__type *)__inst->prev; \ > - __scan_inst != __block->start->prev; \ > + !__scan_inst->is_head_sentinel(); \ > __scan_inst = (__type *)__scan_inst->prev) > > #endif /* BRW_CFG_H */ > diff --git a/src/mesa/drivers/dri/i965/brw_dead_control_flow.cpp > b/src/mesa/drivers/dri/i965/brw_dead_control_flow.cpp > index fad1d42..557c3ad 100644 > --- a/src/mesa/drivers/dri/i965/brw_dead_control_flow.cpp > +++ b/src/mesa/drivers/dri/i965/brw_dead_control_flow.cpp > @@ -47,19 +47,19 @@ dead_control_flow_eliminate(backend_visitor *v) > /* ENDIF instructions, by definition, can only be found at the start of > * basic blocks. > */ > - backend_instruction *endif_inst = endif_block->start; > + backend_instruction *endif_inst = endif_block->start(); > if (endif_inst->opcode != BRW_OPCODE_ENDIF) > continue; > > backend_instruction *if_inst = NULL, *else_inst = NULL; > - backend_instruction *prev_inst = ((bblock_t > *)endif_block->link.prev)->end; > + backend_instruction *prev_inst = ((bblock_t > *)endif_block->link.prev)->end(); > if (prev_inst->opcode == BRW_OPCODE_ELSE) { > else_inst = prev_inst; > else_block = (bblock_t *)endif_block->link.prev; > found = true; > > if (else_block->start_ip == else_block->end_ip) > - prev_inst = ((bblock_t *)else_block->link.prev)->end; > + prev_inst = ((bblock_t *)else_block->link.prev)->end(); > } > > if (prev_inst->opcode == BRW_OPCODE_IF) { > diff --git a/src/mesa/drivers/dri/i965/brw_fs.cpp > b/src/mesa/drivers/dri/i965/brw_fs.cpp > index 39603cd..8e328ea 100644 > --- a/src/mesa/drivers/dri/i965/brw_fs.cpp > +++ b/src/mesa/drivers/dri/i965/brw_fs.cpp > @@ -2230,7 +2230,7 @@ fs_visitor::compute_to_mrf() > * values that end up in MRFs are shortly before the MRF > * write anyway. > */ > - if (block->start == scan_inst) > + if (block->start() == scan_inst) > break; > > /* You can't read from an MRF, so if someone else reads our > @@ -2529,7 +2529,7 @@ > fs_visitor::insert_gen4_pre_send_dependency_workarounds(bblock_t *block, > /* If we hit control flow, assume that there *are* outstanding > * dependencies, and force their cleanup before our instruction. > */ > - if (block->start == scan_inst) { > + if (block->start() == scan_inst) { > for (int i = 0; i < write_len; i++) { > if (needs_dep[i]) { > inst->insert_before(block, DEP_RESOLVE_MOV(first_write_grf + > i)); > @@ -2598,7 +2598,7 @@ > fs_visitor::insert_gen4_post_send_dependency_workarounds(bblock_t *block, > fs_ins > */ > foreach_inst_in_block_starting_from(fs_inst, scan_inst, inst, block) { > /* If we hit control flow, force resolve all remaining dependencies. */ > - if (block->end == scan_inst) { > + if (block->end() == scan_inst) { > for (int i = 0; i < write_len; i++) { > if (needs_dep[i]) > scan_inst->insert_before(block, > diff --git a/src/mesa/drivers/dri/i965/brw_fs_cse.cpp > b/src/mesa/drivers/dri/i965/brw_fs_cse.cpp > index 5615b13..ea8ecf3 100644 > --- a/src/mesa/drivers/dri/i965/brw_fs_cse.cpp > +++ b/src/mesa/drivers/dri/i965/brw_fs_cse.cpp > @@ -254,12 +254,6 @@ fs_visitor::opt_cse_local(bblock_t *block) > fs_inst *prev = (fs_inst *)inst->prev; > > inst->remove(block); > - > - /* Appending an instruction may have changed our bblock end. */ > - if (inst == block->end) { > - block->end = prev; > - } > - > inst = prev; > } > } > diff --git a/src/mesa/drivers/dri/i965/brw_fs_peephole_predicated_break.cpp > b/src/mesa/drivers/dri/i965/brw_fs_peephole_predicated_break.cpp > index 5b4cab5..802b8de 100644 > --- a/src/mesa/drivers/dri/i965/brw_fs_peephole_predicated_break.cpp > +++ b/src/mesa/drivers/dri/i965/brw_fs_peephole_predicated_break.cpp > @@ -52,16 +52,16 @@ fs_visitor::opt_peephole_predicated_break() > /* BREAK and CONTINUE instructions, by definition, can only be found at > * the ends of basic blocks. > */ > - fs_inst *jump_inst = (fs_inst *)block->end; > + fs_inst *jump_inst = (fs_inst *)block->end(); > if (jump_inst->opcode != BRW_OPCODE_BREAK && > jump_inst->opcode != BRW_OPCODE_CONTINUE) > continue; > > - fs_inst *if_inst = (fs_inst *)((bblock_t *)block->link.prev)->end; > + fs_inst *if_inst = (fs_inst *)((bblock_t *)block->link.prev)->end(); > if (if_inst->opcode != BRW_OPCODE_IF) > continue; > > - fs_inst *endif_inst = (fs_inst *)((bblock_t *)block->link.next)->start; > + fs_inst *endif_inst = (fs_inst *)((bblock_t > *)block->link.next)->start(); > if (endif_inst->opcode != BRW_OPCODE_ENDIF) > continue; > > diff --git a/src/mesa/drivers/dri/i965/brw_fs_reg_allocate.cpp > b/src/mesa/drivers/dri/i965/brw_fs_reg_allocate.cpp > index 7e391ea..80c918c 100644 > --- a/src/mesa/drivers/dri/i965/brw_fs_reg_allocate.cpp > +++ b/src/mesa/drivers/dri/i965/brw_fs_reg_allocate.cpp > @@ -200,9 +200,9 @@ count_to_loop_end(bblock_t *block) > for (block = (bblock_t *)block->link.next; > depth > 0; > block = (bblock_t *)block->link.next) { > - if (block->start->opcode == BRW_OPCODE_DO) > + if (block->start()->opcode == BRW_OPCODE_DO) > depth++; > - if (block->end->opcode == BRW_OPCODE_WHILE) { > + if (block->end()->opcode == BRW_OPCODE_WHILE) { > depth--; > if (depth == 0) > return block->end_ip; > diff --git a/src/mesa/drivers/dri/i965/brw_fs_sel_peephole.cpp > b/src/mesa/drivers/dri/i965/brw_fs_sel_peephole.cpp > index 28e4163..2941d65 100644 > --- a/src/mesa/drivers/dri/i965/brw_fs_sel_peephole.cpp > +++ b/src/mesa/drivers/dri/i965/brw_fs_sel_peephole.cpp > @@ -128,14 +128,14 @@ fs_visitor::opt_peephole_sel() > /* IF instructions, by definition, can only be found at the ends of > * basic blocks. > */ > - fs_inst *if_inst = (fs_inst *) block->end; > + fs_inst *if_inst = (fs_inst *)block->end(); > if (if_inst->opcode != BRW_OPCODE_IF) > continue; > > if (!block->else_block) > continue; > > - assert(block->else_block->end->opcode == BRW_OPCODE_ELSE); > + assert(block->else_block->end()->opcode == BRW_OPCODE_ELSE); > > fs_inst *else_mov[MAX_MOVS] = { NULL }; > fs_inst *then_mov[MAX_MOVS] = { NULL }; > diff --git a/src/mesa/drivers/dri/i965/brw_schedule_instructions.cpp > b/src/mesa/drivers/dri/i965/brw_schedule_instructions.cpp > index 0f7d894..b963bda 100644 > --- a/src/mesa/drivers/dri/i965/brw_schedule_instructions.cpp > +++ b/src/mesa/drivers/dri/i965/brw_schedule_instructions.cpp > @@ -631,10 +631,10 @@ instruction_scheduler::add_insts_from_block(bblock_t > *block) > /* Removing the last instruction from a basic block removes the block as > * well, so put a NOP at the end to keep it alive. > */ > - if (!block->end->is_control_flow()) { > + if (!block->end()->is_control_flow()) { > backend_instruction *nop = new(mem_ctx) backend_instruction(); > nop->opcode = BRW_OPCODE_NOP; > - block->end->insert_after(block, nop); > + block->end()->insert_after(block, nop); > } > > foreach_inst_in_block_safe(backend_instruction, inst, block) { > @@ -1370,7 +1370,7 @@ void > instruction_scheduler::schedule_instructions(bblock_t *block) > { > struct brw_context *brw = bv->brw; > - backend_instruction *inst = block->end; > + backend_instruction *inst = block->end(); > time = 0; > > /* Remove non-DAG heads from the list. */ > @@ -1448,8 +1448,8 @@ instruction_scheduler::schedule_instructions(bblock_t > *block) > } > } > > - if (block->end->opcode == BRW_OPCODE_NOP) > - block->end->remove(block); > + if (block->end()->opcode == BRW_OPCODE_NOP) > + block->end()->remove(block); > assert(instructions_to_schedule == 0); > } > > diff --git a/src/mesa/drivers/dri/i965/brw_shader.cpp > b/src/mesa/drivers/dri/i965/brw_shader.cpp > index cbaa278..0298a34 100644 > --- a/src/mesa/drivers/dri/i965/brw_shader.cpp > +++ b/src/mesa/drivers/dri/i965/brw_shader.cpp > @@ -766,9 +766,6 @@ backend_instruction::insert_after(bblock_t *block, > backend_instruction *inst) > > adjust_later_block_ips(block, 1); > > - if (block->end == this) > - block->end = inst; > - > exec_node::insert_after(inst); > } > > @@ -781,9 +778,6 @@ backend_instruction::insert_before(bblock_t *block, > backend_instruction *inst) > > adjust_later_block_ips(block, 1); > > - if (block->start == this) > - block->start = inst; > - > exec_node::insert_before(inst); > } > > @@ -798,9 +792,6 @@ backend_instruction::insert_before(bblock_t *block, > exec_list *list) > > adjust_later_block_ips(block, num_inst); > > - if (block->start == this) > - block->start = (backend_instruction *)list->get_head(); > - > exec_node::insert_before(list); > } > > @@ -815,11 +806,6 @@ backend_instruction::remove(bblock_t *block) > block->cfg->remove_block(block); > } else { > block->end_ip--; > - > - if (block->start == this) > - block->start = (backend_instruction *)this->next; > - if (block->end == this) > - block->end = (backend_instruction *)this->prev; > } > > exec_node::remove(); > diff --git a/src/mesa/drivers/dri/i965/brw_vec4_cse.cpp > b/src/mesa/drivers/dri/i965/brw_vec4_cse.cpp > index e0fb07c..cc0944b 100644 > --- a/src/mesa/drivers/dri/i965/brw_vec4_cse.cpp > +++ b/src/mesa/drivers/dri/i965/brw_vec4_cse.cpp > @@ -184,12 +184,6 @@ vec4_visitor::opt_cse_local(bblock_t *block) > vec4_instruction *prev = (vec4_instruction *)inst->prev; > > inst->remove(block); > - > - /* Appending an instruction may have changed our bblock end. */ > - if (inst == block->end) { > - block->end = prev; > - } > - > inst = prev; > } > } > diff --git a/src/mesa/drivers/dri/i965/intel_asm_annotation.c > b/src/mesa/drivers/dri/i965/intel_asm_annotation.c > index 953a60a..37ad090 100644 > --- a/src/mesa/drivers/dri/i965/intel_asm_annotation.c > +++ b/src/mesa/drivers/dri/i965/intel_asm_annotation.c > @@ -114,7 +114,7 @@ void annotate(struct brw_context *brw, > ann->annotation = inst->annotation; > } > > - if (cfg->blocks[annotation->cur_block]->start == inst) { > + if (bblock_start(cfg->blocks[annotation->cur_block]) == inst) { > ann->block_start = cfg->blocks[annotation->cur_block]; > } > > @@ -130,7 +130,7 @@ void annotate(struct brw_context *brw, > annotation->ann_count--; > } > > - if (cfg->blocks[annotation->cur_block]->end == inst) { > + if (bblock_end(cfg->blocks[annotation->cur_block]) == inst) { > ann->block_end = cfg->blocks[annotation->cur_block]; > annotation->cur_block++; > } > -- > 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