[Mesa-stable] [Mesa-dev] [PATCH 2/3] i965/fs: Use fs_inst::overwrites_reg() in saturate propagation.

[Ian Romanick]

On 02/11/2015 02:54 PM, Matt Turner wrote:
> This is safer and matches the conditional_mod propagation pass.
> 
> Cc: <mesa-stable at lists.freedesktop.org>
> ---
>  .../dri/i965/brw_fs_saturate_propagation.cpp       |  8 ++---
>  .../dri/i965/test_fs_saturate_propagation.cpp      | 40 ++++++++++++++++++++++
>  2 files changed, 44 insertions(+), 4 deletions(-)
> 
> diff --git a/src/mesa/drivers/dri/i965/brw_fs_saturate_propagation.cpp b/src/mesa/drivers/dri/i965/brw_fs_saturate_propagation.cpp
> index a9966a4..bc51661 100644
> --- a/src/mesa/drivers/dri/i965/brw_fs_saturate_propagation.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_fs_saturate_propagation.cpp
> @@ -64,10 +64,10 @@ opt_saturate_propagation_local(fs_visitor *v, bblock_t *block)
>  
>        bool interfered = false;
>        foreach_inst_in_block_reverse_starting_from(fs_inst, scan_inst, inst, block) {
> -         if (scan_inst->dst.file == GRF &&
> -             scan_inst->dst.reg == inst->src[0].reg &&
> -             scan_inst->dst.reg_offset == inst->src[0].reg_offset &&
> -             !scan_inst->is_partial_write()) {
> +         if (scan_inst->overwrites_reg(inst->src[0])) {
> +            if (scan_inst->is_partial_write())
> +               break;
> +
>              if (scan_inst->saturate) {
>                 inst->saturate = false;
>                 progress = true;
> diff --git a/src/mesa/drivers/dri/i965/test_fs_saturate_propagation.cpp b/src/mesa/drivers/dri/i965/test_fs_saturate_propagation.cpp
> index 2000830..f897bdd 100644
> --- a/src/mesa/drivers/dri/i965/test_fs_saturate_propagation.cpp
> +++ b/src/mesa/drivers/dri/i965/test_fs_saturate_propagation.cpp
> @@ -353,3 +353,43 @@ TEST_F(saturate_propagation_test, intervening_saturating_copy)
>     EXPECT_EQ(BRW_OPCODE_MOV, instruction(block0, 2)->opcode);
>     EXPECT_FALSE(instruction(block0, 2)->saturate);
>  }
> +
> +TEST_F(saturate_propagation_test, intervening_dest_write)

Does this test pass without the other change?

> +{
> +   fs_reg dst0 = v->vgrf(glsl_type::vec4_type);
> +   fs_reg dst1 = v->vgrf(glsl_type::float_type);
> +   fs_reg src0 = v->vgrf(glsl_type::float_type);
> +   fs_reg src1 = v->vgrf(glsl_type::float_type);
> +   fs_reg src2 = v->vgrf(glsl_type::vec2_type);
> +   v->emit(BRW_OPCODE_ADD, offset(dst0, 2), src0, src1);
> +   v->emit(SHADER_OPCODE_TEX, dst0, src2)
> +      ->regs_written = 4;
> +   v->emit(BRW_OPCODE_MOV, dst1, offset(dst0, 2))
> +      ->saturate = true;
> +
> +   /* = Before =
> +    *
> +    * 0: add(8)        dst0+2  src0    src1
> +    * 1: tex(8) rlen 4 dst0+0  src2
> +    * 2: mov.sat(8)    dst1    dst0+2
> +    *
> +    * = After =
> +    * (no changes)
> +    */
> +
> +   v->calculate_cfg();
> +   bblock_t *block0 = v->cfg->blocks[0];
> +
> +   EXPECT_EQ(0, block0->start_ip);
> +   EXPECT_EQ(2, block0->end_ip);
> +
> +   EXPECT_FALSE(saturate_propagation(v));
> +   EXPECT_EQ(0, block0->start_ip);
> +   EXPECT_EQ(2, block0->end_ip);
> +   EXPECT_EQ(BRW_OPCODE_ADD, instruction(block0, 0)->opcode);
> +   EXPECT_FALSE(instruction(block0, 0)->saturate);
> +   EXPECT_EQ(SHADER_OPCODE_TEX, instruction(block0, 1)->opcode);
> +   EXPECT_FALSE(instruction(block0, 0)->saturate);
> +   EXPECT_EQ(BRW_OPCODE_MOV, instruction(block0, 2)->opcode);
> +   EXPECT_TRUE(instruction(block0, 2)->saturate);
> +}
>