On Tue, Aug 19, 2014 at 1:52 AM, Pohjolainen, Topi
<topi.pohjolai...@intel.com> wrote:
> 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").
Yes, exactly. I went back and forth about how to do this.
>> + 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.
Yep. Thanks for the review!
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
