[Mesa-dev] [PATCH] i965: Don't mark dead instructions' sources live.

[Kenneth Graunke]

On Wednesday, November 25, 2015 06:14:28 PM Matt Turner wrote:
> Removes dead code from glsl-mat-from-int-ctor-03.shader_test.
> 
> Reported-by: Juan A. Suarez Romero <jasuarez at igalia.com>
> ---
>  src/mesa/drivers/dri/i965/brw_fs_dead_code_eliminate.cpp   | 4 ++++
>  src/mesa/drivers/dri/i965/brw_vec4_dead_code_eliminate.cpp | 4 ++++
>  2 files changed, 8 insertions(+)
> 
> diff --git a/src/mesa/drivers/dri/i965/brw_fs_dead_code_eliminate.cpp b/src/mesa/drivers/dri/i965/brw_fs_dead_code_eliminate.cpp
> index a50cf6f..6b4b602 100644
> --- a/src/mesa/drivers/dri/i965/brw_fs_dead_code_eliminate.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_fs_dead_code_eliminate.cpp
> @@ -109,6 +109,10 @@ fs_visitor::dead_code_eliminate()
>              BITSET_CLEAR(flag_live, inst->flag_subreg);
>           }
>  
> +         /* Don't mark dead instructions' sources as live */
> +         if (inst->opcode == BRW_OPCODE_NOP)
> +            continue;
> +
>           for (int i = 0; i < inst->sources; i++) {
>              if (inst->src[i].file == VGRF) {
>                 int var = live_intervals->var_from_reg(inst->src[i]);
> diff --git a/src/mesa/drivers/dri/i965/brw_vec4_dead_code_eliminate.cpp b/src/mesa/drivers/dri/i965/brw_vec4_dead_code_eliminate.cpp
> index 58aed81..369941b 100644
> --- a/src/mesa/drivers/dri/i965/brw_vec4_dead_code_eliminate.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_vec4_dead_code_eliminate.cpp
> @@ -150,6 +150,10 @@ vec4_visitor::dead_code_eliminate()
>                 BITSET_CLEAR(flag_live, c);
>           }
>  
> +         /* Don't mark dead instructions' sources as live */
> +         if (inst->opcode == BRW_OPCODE_NOP)
> +            continue;
> +
>           for (int i = 0; i < 3; i++) {
>              if (inst->src[i].file == VGRF) {
>                 for (unsigned j = 0; j < inst->regs_read(i); j++) {
> 

This is
Reviewed-by: Kenneth Graunke <kenneth at whitecape.org>

A while back I grumbled at you and Tapani about not updating
inst->sources and leaving junk source registers in place when mutating
an instruction to change its opcode.

If inst->sources were updated correctly when converting to NOP (i.e. set
to 0), you would not have this bug, as the loop over sources would not
happen.  You wouldn't need to special case NOP here, either.

Which actually makes me wonder: if we convert from ADD to MOV (or
similar), do we leave a bogus src[1] in place and treat it as live?
Presumably most cases where that happens are because we did algebraic
optimizations involving IMMs, so the extra live thing would be an
immediate and not cause any trouble...

--Ken
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: This is a digitally signed message part.
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20151127/796f39e4/attachment.sig>