[Mesa-dev] [PATCH 13/20] i965/fs: Don't iterate between blocks with inst->next/prev.
Pohjolainen, Topi
topi.pohjolainen at intel.com
Wed Sep 17 05:55:48 PDT 2014
On Tue, Sep 02, 2014 at 09:34:24PM -0700, Matt Turner wrote:
> When instruction lists are per-basic block, this won't work.
> ---
> .../drivers/dri/i965/brw_dead_control_flow.cpp | 5 +--
> src/mesa/drivers/dri/i965/brw_fs.cpp | 20 ++++-------
> .../dri/i965/brw_fs_peephole_predicated_break.cpp | 7 ++--
> .../dri/i965/brw_fs_saturate_propagation.cpp | 7 +---
> src/mesa/drivers/dri/i965/brw_fs_sel_peephole.cpp | 42 ++++++++++------------
> 5 files changed, 34 insertions(+), 47 deletions(-)
Patches 12 and 13 are:
Reviewed-by: Topi Pohjolainen <topi.pohjolainen at intel.com>
I'm only thinking if the names for
foreach_inst_in_block_reverse_starting_from() and
foreach_inst_in_block_starting_from() should reflect the fact that they
start from the previous and next instructions respectively (instead of the one
given as the starting point). Or if the callers should give
inst->prev/inst->next as the starting point instead.
>
> 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 485ab91..fad1d42 100644
> --- a/src/mesa/drivers/dri/i965/brw_dead_control_flow.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_dead_control_flow.cpp
> @@ -52,13 +52,14 @@ dead_control_flow_eliminate(backend_visitor *v)
> continue;
>
> backend_instruction *if_inst = NULL, *else_inst = NULL;
> - backend_instruction *prev_inst = (backend_instruction *) endif_inst->prev;
> + 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;
>
> - prev_inst = (backend_instruction *) prev_inst->prev;
> + if (else_block->start_ip == else_block->end_ip)
> + 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 8faa930..5277420 100644
> --- a/src/mesa/drivers/dri/i965/brw_fs.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_fs.cpp
> @@ -2185,10 +2185,7 @@ fs_visitor::compute_to_mrf()
> /* Found a move of a GRF to a MRF. Let's see if we can go
> * rewrite the thing that made this GRF to write into the MRF.
> */
> - fs_inst *scan_inst;
> - for (scan_inst = (fs_inst *)inst->prev;
> - !scan_inst->is_head_sentinel();
> - scan_inst = (fs_inst *)scan_inst->prev) {
> + foreach_inst_in_block_reverse_starting_from(fs_inst, scan_inst, inst, block) {
> if (scan_inst->dst.file == GRF &&
> scan_inst->dst.reg == inst->src[0].reg) {
> /* Found the last thing to write our reg we want to turn
> @@ -2237,7 +2234,7 @@ fs_visitor::compute_to_mrf()
> * values that end up in MRFs are shortly before the MRF
> * write anyway.
> */
> - if (scan_inst->is_control_flow() && scan_inst->opcode != BRW_OPCODE_IF)
> + if (block->start == scan_inst)
> break;
>
> /* You can't read from an MRF, so if someone else reads our
> @@ -2532,14 +2529,11 @@ fs_visitor::insert_gen4_pre_send_dependency_workarounds(bblock_t *block,
> * we assume that there are no outstanding dependencies on entry to the
> * program.
> */
> - for (fs_inst *scan_inst = (fs_inst *)inst->prev;
> - !scan_inst->is_head_sentinel();
> - scan_inst = (fs_inst *)scan_inst->prev) {
> -
> + foreach_inst_in_block_reverse_starting_from(fs_inst, scan_inst, inst, block) {
> /* If we hit control flow, assume that there *are* outstanding
> * dependencies, and force their cleanup before our instruction.
> */
> - if (scan_inst->is_control_flow()) {
> + 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));
> @@ -2606,11 +2600,9 @@ fs_visitor::insert_gen4_post_send_dependency_workarounds(bblock_t *block, fs_ins
> /* Walk forwards looking for writes to registers we're writing which aren't
> * read before being written.
> */
> - for (fs_inst *scan_inst = (fs_inst *)inst->next;
> - !scan_inst->is_tail_sentinel();
> - scan_inst = (fs_inst *)scan_inst->next) {
> + foreach_inst_in_block_starting_from(fs_inst, scan_inst, inst, block) {
> /* If we hit control flow, force resolve all remaining dependencies. */
> - if (scan_inst->is_control_flow()) {
> + 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_peephole_predicated_break.cpp b/src/mesa/drivers/dri/i965/brw_fs_peephole_predicated_break.cpp
> index c669fbe..5b4cab5 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
> @@ -46,6 +46,9 @@ fs_visitor::opt_peephole_predicated_break()
> bool progress = false;
>
> foreach_block (block, cfg) {
> + if (block->start_ip != block->end_ip)
> + continue;
> +
> /* BREAK and CONTINUE instructions, by definition, can only be found at
> * the ends of basic blocks.
> */
> @@ -54,11 +57,11 @@ fs_visitor::opt_peephole_predicated_break()
> jump_inst->opcode != BRW_OPCODE_CONTINUE)
> continue;
>
> - fs_inst *if_inst = (fs_inst *)jump_inst->prev;
> + 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 *)jump_inst->next;
> + 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_saturate_propagation.cpp b/src/mesa/drivers/dri/i965/brw_fs_saturate_propagation.cpp
> index d65b2f1..4c4b6bf 100644
> --- a/src/mesa/drivers/dri/i965/brw_fs_saturate_propagation.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_fs_saturate_propagation.cpp
> @@ -48,13 +48,8 @@ opt_saturate_propagation_local(fs_visitor *v, bblock_t *block)
> int src_var = v->live_intervals->var_from_reg(&inst->src[0]);
> int src_end_ip = v->live_intervals->end[src_var];
>
> - int scan_ip = ip;
> bool interfered = false;
> - for (fs_inst *scan_inst = (fs_inst *) inst->prev;
> - scan_inst != block->start->prev;
> - scan_inst = (fs_inst *) scan_inst->prev) {
> - scan_ip--;
> -
> + foreach_inst_in_block_reverse_starting_from(fs_inst, scan_inst, inst, block) {
> if (scan_inst->dst.file == GRF &&
> scan_inst->dst.reg == inst->src[0].reg &&
> scan_inst->dst.reg_offset == inst->src[0].reg_offset &&
> 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 2437a2f..28e4163 100644
> --- a/src/mesa/drivers/dri/i965/brw_fs_sel_peephole.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_fs_sel_peephole.cpp
> @@ -41,9 +41,9 @@
> * Scans forwards from an IF counting consecutive MOV instructions in the
> * "then" and "else" blocks of the if statement.
> *
> - * A pointer to the fs_inst* for IF is passed as the <if_inst> argument. The
> - * function stores pointers to the MOV instructions in the <then_mov> and
> - * <else_mov> arrays.
> + * A pointer to the bblock_t following the IF is passed as the <then_block>
> + * argument. The function stores pointers to the MOV instructions in the
> + * <then_mov> and <else_mov> arrays.
> *
> * \return the minimum number of MOVs found in the two branches or zero if
> * an error occurred.
> @@ -62,26 +62,23 @@
> */
> static int
> count_movs_from_if(fs_inst *then_mov[MAX_MOVS], fs_inst *else_mov[MAX_MOVS],
> - fs_inst *if_inst, fs_inst *else_inst)
> + bblock_t *then_block, bblock_t *else_block)
> {
> - fs_inst *m = if_inst;
> -
> - assert(m->opcode == BRW_OPCODE_IF);
> - m = (fs_inst *) m->next;
> -
> int then_movs = 0;
> - while (then_movs < MAX_MOVS && m->opcode == BRW_OPCODE_MOV) {
> - then_mov[then_movs] = m;
> - m = (fs_inst *) m->next;
> + foreach_inst_in_block(fs_inst, inst, then_block) {
> + if (then_movs == MAX_MOVS || inst->opcode != BRW_OPCODE_MOV)
> + break;
> +
> + then_mov[then_movs] = inst;
> then_movs++;
> }
>
> - m = (fs_inst *) else_inst->next;
> -
> int else_movs = 0;
> - while (else_movs < MAX_MOVS && m->opcode == BRW_OPCODE_MOV) {
> - else_mov[else_movs] = m;
> - m = (fs_inst *) m->next;
> + foreach_inst_in_block(fs_inst, inst, else_block) {
> + if (else_movs == MAX_MOVS || inst->opcode != BRW_OPCODE_MOV)
> + break;
> +
> + else_mov[else_movs] = inst;
> else_movs++;
> }
>
> @@ -138,13 +135,15 @@ fs_visitor::opt_peephole_sel()
> if (!block->else_block)
> continue;
>
> - fs_inst *else_inst = (fs_inst *) block->else_block->end;
> - assert(else_inst->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 };
>
> - int movs = count_movs_from_if(then_mov, else_mov, if_inst, else_inst);
> + bblock_t *then_block = (bblock_t *)block->link.next;
> + bblock_t *else_block = (bblock_t *)block->else_block->link.next;
> +
> + int movs = count_movs_from_if(then_mov, else_mov, then_block, else_block);
>
> if (movs == 0)
> continue;
> @@ -213,9 +212,6 @@ fs_visitor::opt_peephole_sel()
> if_inst->insert_before(block, cmp_inst);
> }
>
> - bblock_t *then_block = (bblock_t *)block->link.next;
> - bblock_t *else_block = (bblock_t *)block->else_block->link.next;
> -
> for (int i = 0; i < movs; i++) {
> if (mov_imm_inst[i])
> if_inst->insert_before(block, mov_imm_inst[i]);
> --
> 1.8.5.5
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
More information about the mesa-dev
mailing list