[Mesa-dev] [PATCH] i965/vec4: Don't disable channels in any/all comparisons.

[Jason Ekstrand]

On Oct 29, 2015 5:51 PM, "Matt Turner" <mattst88 at gmail.com> wrote:
>
> We've made a mistake in calling the Channel Enable bits "writemask",
> because they do more than control which channels of the destination are
> written -- they actually control which channels are enabled (surprise!
> surprise!)
>
> So, if we emit
>
>                cmp.z.f0(8) null.xy<1>D  g10<4,4,1>.xyzzD g2<0,4,1>.xyzzD
>                mov(8)      g12<1>.xUD   0x00000000UD
>    (+f0.all4h) mov(8)      g12<1>.xUD   0xffffffffUD
>
> where the CMP instruction has only .xy channel enables, it won't write
> the .zw channels of the flag register, which are of course read by the
> +f0.all4 predicate.
>
> We need to always emit CMP instructions whose flag result might be read
> by such a predicate with all channels enabled.

Makes sense.

Reviewed-by: Jason Ekstrand <jason.ekstrand at intel.com>

> ---
>  src/mesa/drivers/dri/i965/brw_vec4_nir.cpp | 52
++++++------------------------
>  1 file changed, 10 insertions(+), 42 deletions(-)
>
> diff --git a/src/mesa/drivers/dri/i965/brw_vec4_nir.cpp
b/src/mesa/drivers/dri/i965/brw_vec4_nir.cpp
> index 0f04f65..33cc02e 100644
> --- a/src/mesa/drivers/dri/i965/brw_vec4_nir.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_vec4_nir.cpp
> @@ -1146,26 +1146,10 @@ vec4_visitor::nir_emit_alu(nir_alu_instr *instr)
>     case nir_op_ball_iequal3:
>     case nir_op_ball_fequal4:
>     case nir_op_ball_iequal4: {
> -      dst_reg tmp = dst_reg(this, glsl_type::bool_type);
> +      unsigned swiz =
> +         brw_swizzle_for_size(nir_op_infos[instr->op].input_sizes[0]);
>
> -      switch (instr->op) {
> -      case nir_op_ball_fequal2:
> -      case nir_op_ball_iequal2:
> -         tmp.writemask = WRITEMASK_XY;
> -         break;
> -      case nir_op_ball_fequal3:
> -      case nir_op_ball_iequal3:
> -         tmp.writemask = WRITEMASK_XYZ;
> -         break;
> -      case nir_op_ball_fequal4:
> -      case nir_op_ball_iequal4:
> -         tmp.writemask = WRITEMASK_XYZW;
> -         break;
> -      default:
> -         unreachable("not reached");
> -      }
> -
> -      emit(CMP(tmp, op[0], op[1],
> +      emit(CMP(dst_null_d(), swizzle(op[0], swiz), swizzle(op[1], swiz),
>                 brw_conditional_for_nir_comparison(instr->op)));
>        emit(MOV(dst, src_reg(0)));
>        inst = emit(MOV(dst, src_reg(~0)));
> @@ -1179,26 +1163,10 @@ vec4_visitor::nir_emit_alu(nir_alu_instr *instr)
>     case nir_op_bany_inequal3:
>     case nir_op_bany_fnequal4:
>     case nir_op_bany_inequal4: {
> -      dst_reg tmp = dst_reg(this, glsl_type::bool_type);
> +      unsigned swiz =
> +         brw_swizzle_for_size(nir_op_infos[instr->op].input_sizes[0]);
>
> -      switch (instr->op) {
> -      case nir_op_bany_fnequal2:
> -      case nir_op_bany_inequal2:
> -         tmp.writemask = WRITEMASK_XY;
> -         break;
> -      case nir_op_bany_fnequal3:
> -      case nir_op_bany_inequal3:
> -         tmp.writemask = WRITEMASK_XYZ;
> -         break;
> -      case nir_op_bany_fnequal4:
> -      case nir_op_bany_inequal4:
> -         tmp.writemask = WRITEMASK_XYZW;
> -         break;
> -      default:
> -         unreachable("not reached");
> -      }
> -
> -      emit(CMP(tmp, op[0], op[1],
> +      emit(CMP(dst_null_d(), swizzle(op[0], swiz), swizzle(op[1], swiz),
>                 brw_conditional_for_nir_comparison(instr->op)));
>
>        emit(MOV(dst, src_reg(0)));
> @@ -1463,11 +1431,11 @@ vec4_visitor::nir_emit_alu(nir_alu_instr *instr)
>     case nir_op_bany2:
>     case nir_op_bany3:
>     case nir_op_bany4: {
> -      dst_reg tmp = dst_reg(this, glsl_type::bool_type);
> -      tmp.writemask =
brw_writemask_for_size(nir_op_infos[instr->op].input_sizes[0]);
> -
> -      emit(CMP(tmp, op[0], src_reg(0), BRW_CONDITIONAL_NZ));
> +      unsigned swiz =
> +         brw_swizzle_for_size(nir_op_infos[instr->op].input_sizes[0]);
>
> +      emit(CMP(dst_null_d(), swizzle(op[0], swiz), src_reg(0),
> +               BRW_CONDITIONAL_NZ));
>        emit(MOV(dst, src_reg(0)));
>        inst = emit(MOV(dst, src_reg(~0)));
>        inst->predicate = BRW_PREDICATE_ALIGN16_ANY4H;
> --
> 2.4.9
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20151029/615a7c08/attachment.html>