[Mesa-dev] [PATCH 3/5] i965/cfg: Eliminate an empty then-branch of an if/else/endif

[Matt Turner]

On Thu, Feb 25, 2016 at 5:56 PM, Francisco Jerez <currojerez at riseup.net> wrote:
> Ian Romanick <idr at freedesktop.org> writes:
>
>> From: Ian Romanick <ian.d.romanick at intel.com>
>>
>> On BDW,
>>
>> total instructions in shared programs: 8448571 -> 8448367 (-0.00%)
>> instructions in affected programs: 21000 -> 20796 (-0.97%)
>> helped: 116
>> HURT: 0
>>
>> Signed-off-by: Ian Romanick <ian.d.romanick at intel.com>
>> ---
>>  src/mesa/drivers/dri/i965/brw_dead_control_flow.cpp | 18 ++++++++++++++++++
>>  1 file changed, 18 insertions(+)
>>
>> 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 7aa72b1..149596f 100644
>> --- a/src/mesa/drivers/dri/i965/brw_dead_control_flow.cpp
>> +++ b/src/mesa/drivers/dri/i965/brw_dead_control_flow.cpp
>> @@ -34,6 +34,7 @@
>>   *   - if/endif
>>   *   . else in else/endif
>>   *   - if/else/endif
>> + *   - then in if/else/endif
>>   */
>>  bool
>>  dead_control_flow_eliminate(backend_shader *s)
>> @@ -114,6 +115,23 @@ dead_control_flow_eliminate(backend_shader *s)
>>
>>              progress = true;
>>           }
>> +      } else if (inst->opcode == BRW_OPCODE_ELSE &&
>> +                 prev_inst->opcode == BRW_OPCODE_IF) {
>> +         bblock_t *const else_block = block;
>> +         bblock_t *const if_block = prev_block;
>> +         backend_instruction *const if_inst = prev_inst;
>> +         backend_instruction *const else_inst = inst;
>> +
>> +         /* Since the else-branch is becoming the new then-branch, the
>> +          * condition has to be inverted.
>> +          */
>> +         if_inst->predicate_inverse = !if_inst->predicate_inverse;
>> +         else_inst->remove(else_block);
>> +
>> +         if (if_block->can_combine_with(else_block))
>> +            if_block->combine_with(else_block);
>
> Ugh, IIRC backend_instruction::remove(block) will remove the block
> behind your back when it becomes empty (and it will here because ELSE
> can only be the only instruction left inside 'block' whenever you hit
> this path), so you're passing a pointer to a no-longer-existing block to
> (can_)combine_with().  I believe this will never let you combine the
> blocks anyway because the previous block ends with an IF instruction
> which you haven't removed, so this is effectively a no-op [assuming it
> doesn't crash ;)].  If you drop the two lines above you can put my

Exactly right.