On Thu, Jul 24, 2014 at 07:54:27PM -0700, Matt Turner wrote:
> Operating on this code,
>
> B0: ...
> cmp.ne.f0(8)
> (+f0) if(8)
> B1: break(8)
> B2: endif(8)
>
> We can delete B2 without attempting to merge any blocks, since the
> break/continue instruction necessarily ends the previous block.
>
> After deleting the if instruction, we attempt to merge blocks B0 and B1.
> ---
> .../dri/i965/brw_fs_peephole_predicated_break.cpp | 29
> +++++++++++++++++++---
> 1 file changed, 25 insertions(+), 4 deletions(-)
>
> 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 445d10e..ab197ee 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
> @@ -64,27 +64,48 @@ fs_visitor::opt_peephole_predicated_break()
> if (endif_inst->opcode != BRW_OPCODE_ENDIF)
> continue;
>
> + bblock_t *jump_block = block;
> + bblock_t *if_block = (bblock_t *)jump_block->link.prev;
> + bblock_t *endif_block = (bblock_t *)jump_block->link.next;
> +
> /* For Sandybridge with IF with embedded comparison we need to emit an
> * instruction to set the flag register.
> */
> if (brw->gen == 6 && if_inst->conditional_mod) {
> fs_inst *cmp_inst = CMP(reg_null_d, if_inst->src[0],
> if_inst->src[1],
> if_inst->conditional_mod);
> - if_inst->insert_before(cmp_inst);
> + if_inst->insert_before(if_block, cmp_inst);
> jump_inst->predicate = BRW_PREDICATE_NORMAL;
> } else {
> jump_inst->predicate = if_inst->predicate;
> jump_inst->predicate_inverse = if_inst->predicate_inverse;
> }
>
> - if_inst->remove();
> - endif_inst->remove();
> + bblock_t *earlier_block = if_block;
> + if (if_block->start_ip == if_block->end_ip) {
> + earlier_block = (bblock_t *)if_block->link.prev;
> + }
> +
> + if_inst->remove(if_block);
> + endif_inst->remove(endif_block);
> +
> + if_block->children.make_empty();
> + endif_block->parents.make_empty();
> +
> + if_block->add_successor(cfg->mem_ctx, jump_block);
I'm just checking how well I understood things. As we don't have a method
remove_successor() we need to call make_empty() and then add jump_block
again as a successor. It was already a successor before we called make_empty(),
right? (Above we have "if_block = (bblock_t *)jump_block->link.prev").
> + jump_block->add_successor(cfg->mem_ctx, endif_block);
And the same thing applies here, we called make_empty() to clear the parent
list of endif_block so we need to associate jump_block and endif_block again.
Given that this is how it works:
Reviewed-by: Topi Pohjolainen <topi.pohjolai...@intel.com>
> +
> + if (earlier_block->can_combine_with(jump_block)) {
> + earlier_block->combine_with(jump_block);
> +
> + block = earlier_block;
> + }
>
> progress = true;
> }
>
> if (progress)
> - invalidate_live_intervals();
> + invalidate_live_intervals(false);
>
> return progress;
> }
> --
> 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
