[Mesa-dev] [PATCH 02/12] i965/vec4: Don't cmod propagate from CMP to ADD if the writemask isn't compatible

[Caio Marcelo de Oliveira Filho]

Patch looks good to me, consider suggestion in the end of the email.

Reviewed-by: Caio Marcelo de Oliveira Filho <caio.oliveira at intel.com>


A related question: for the case "if (inst->opcode == BRW_OPCODE_CMP
&& !inst->src[1].is_zero())" don't we need to break if we find an ADD
that is not a match but writes to the register we use in comparison?
In the general case this is covered by the regions_overlap.

E.g.

    0: add        dest  src0  src1
    1: add        src0  src2  src3
    2: cmp.ge.f0  null  src0  -src1

When scanning with inst=2 and scan_inst=1, the current code "goto not
match", and will keep scanning to scan_inst=0. Should it "break"
instead?


> diff --git a/src/intel/compiler/brw_vec4_cmod_propagation.cpp b/src/intel/compiler/brw_vec4_cmod_propagation.cpp
> index 9560cc3b6f7..0602e25ffc2 100644
> --- a/src/intel/compiler/brw_vec4_cmod_propagation.cpp
> +++ b/src/intel/compiler/brw_vec4_cmod_propagation.cpp
> @@ -82,6 +82,14 @@ opt_cmod_propagation_local(bblock_t *block, vec4_visitor *v)
>              if (scan_inst->opcode != BRW_OPCODE_ADD)
>                 goto not_match;
>  
> +            if ((scan_inst->dst.writemask != WRITEMASK_X &&
> +                 scan_inst->dst.writemask != WRITEMASK_XYZW) ||
> +                (scan_inst->dst.writemask == WRITEMASK_XYZW &&
> +                 inst->src[0].swizzle != BRW_SWIZZLE_XYZW) ||
> +                (inst->dst.writemask & ~scan_inst->dst.writemask) != 0) {
> +               goto not_match;
> +            }
> +

It could be worth to capture these conditions (and maybe the exec_size
/ group matching) into a helper function (or a local boolean earlier
in the code) to be used by this and the next block that handle the
case when regions overlap.


Thanks,
Caio