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

[Francisco Jerez]

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

Reviewed-by: Francisco Jerez <currojerez at riseup.net>

on this patch and the rest of the series.

> +
> +         progress = true;
>        }
>     }
>  
> -- 
> 2.5.0
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 212 bytes
Desc: not available
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20160225/15b255f1/attachment.sig>