Patch looks good to me, consider suggestion in the end of the email.
Reviewed-by: Caio Marcelo de Oliveira Filho <caio.olive...@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
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
