[Mesa-dev] [PATCH 04/13] i965: Mask out unused Align16 components in brw_untyped_atomic.

[Pohjolainen, Topi]

On Fri, Feb 27, 2015 at 05:34:47PM +0200, Francisco Jerez wrote:
> This is currently not a problem because the vec4 visitor happens to
> mask out unused components from the destination, but it might become
> an issue when we start using atomics without writeback message.  In
> any case it seems sensible to set it again here because the
> consequences of setting the wrong writemask (random graphics memory
> corruption) are difficult to debug and can easily go unnoticed.

I started thinking if this should be an assertion here and should we force
the logic in the visitor to consider the writemask correctly instead? I don't
have a strong opinion, merely just wondering aloud.

> ---
>  src/mesa/drivers/dri/i965/brw_eu_emit.c | 13 +++++++++++--
>  1 file changed, 11 insertions(+), 2 deletions(-)
> 
> diff --git a/src/mesa/drivers/dri/i965/brw_eu_emit.c b/src/mesa/drivers/dri/i965/brw_eu_emit.c
> index 2b1d6ff..0b655d4 100644
> --- a/src/mesa/drivers/dri/i965/brw_eu_emit.c
> +++ b/src/mesa/drivers/dri/i965/brw_eu_emit.c
> @@ -2799,16 +2799,25 @@ brw_untyped_atomic(struct brw_compile *p,
>                     bool response_expected)
>  {
>     const struct brw_context *brw = p->brw;
> +   const bool align1 = (brw_inst_access_mode(brw, p->current) == BRW_ALIGN_1);
> +   /* Mask out unused components -- This is especially important in Align16
> +    * mode on generations that don't have native support for SIMD4x2 atomics,
> +    * because unused but enabled components will cause the dataport to perform
> +    * additional atomic operations on the addresses that happen to be in the
> +    * uninitialized Y, Z and W coordinates of the payload.
> +    */
> +   const unsigned mask = (align1 ? WRITEMASK_XYZW : WRITEMASK_X);
>     brw_inst *insn = brw_next_insn(p, BRW_OPCODE_SEND);
>  
> -   brw_set_dest(p, insn, retype(dest, BRW_REGISTER_TYPE_UD));
> +   brw_set_dest(p, insn, retype(brw_writemask(dest, mask),
> +                                BRW_REGISTER_TYPE_UD));
>     brw_set_src0(p, insn, retype(payload, BRW_REGISTER_TYPE_UD));
>     brw_set_src1(p, insn, brw_imm_d(0));
>     brw_set_dp_untyped_atomic_message(
>        p, insn, atomic_op, bind_table_index, msg_length,
>        brw_surface_payload_size(p, response_expected,
>                                 brw->gen >= 8 || brw->is_haswell, true),
> -      brw_inst_access_mode(brw, insn) == BRW_ALIGN_1);
> +      align1);
>  }
>  
>  static void
> -- 
> 2.1.3
> 
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev