[Mesa-dev] [PATCH 05/13] i965: Fix the untyped surface opcodes to deal with indirect surface access.

[Pohjolainen, Topi]

On Fri, Feb 27, 2015 at 05:34:48PM +0200, Francisco Jerez wrote:
> Change brw_untyped_atomic() and brw_untyped_surface_read() to take the
> surface index as a register instead of a constant and to use
> brw_send_indirect_message() to emit the indirect variant of send with
> a dynamically calculated message descriptor.  This will be required to
> support variable indexing of image arrays for
> ARB_shader_image_load_store.
> ---
>  src/mesa/drivers/dri/i965/brw_eu.h               |  10 +-
>  src/mesa/drivers/dri/i965/brw_eu_emit.c          | 158 +++++++++++++----------
>  src/mesa/drivers/dri/i965/brw_fs_generator.cpp   |   4 +-
>  src/mesa/drivers/dri/i965/brw_vec4_generator.cpp |   4 +-
>  4 files changed, 96 insertions(+), 80 deletions(-)
> 
> diff --git a/src/mesa/drivers/dri/i965/brw_eu.h b/src/mesa/drivers/dri/i965/brw_eu.h
> index 87a9f3f..9cc9123 100644
> --- a/src/mesa/drivers/dri/i965/brw_eu.h
> +++ b/src/mesa/drivers/dri/i965/brw_eu.h
> @@ -398,18 +398,18 @@ void brw_CMP(struct brw_compile *p,
>  
>  void
>  brw_untyped_atomic(struct brw_compile *p,
> -                   struct brw_reg dest,
> +                   struct brw_reg dst,
>                     struct brw_reg payload,
> +                   struct brw_reg surface,
>                     unsigned atomic_op,
> -                   unsigned bind_table_index,
>                     unsigned msg_length,
>                     bool response_expected);
>  
>  void
>  brw_untyped_surface_read(struct brw_compile *p,
> -                         struct brw_reg dest,
> -                         struct brw_reg mrf,
> -                         unsigned bind_table_index,
> +                         struct brw_reg dst,
> +                         struct brw_reg payload,
> +                         struct brw_reg surface,
>                           unsigned msg_length,
>                           unsigned num_channels);
>  
> diff --git a/src/mesa/drivers/dri/i965/brw_eu_emit.c b/src/mesa/drivers/dri/i965/brw_eu_emit.c
> index 0b655d4..34695bf 100644
> --- a/src/mesa/drivers/dri/i965/brw_eu_emit.c
> +++ b/src/mesa/drivers/dri/i965/brw_eu_emit.c
> @@ -2518,6 +2518,48 @@ brw_send_indirect_message(struct brw_compile *p,
>     return setup;
>  }
>  
> +static struct brw_inst *
> +brw_send_indirect_surface_message(struct brw_compile *p,
> +                                  unsigned sfid,
> +                                  struct brw_reg dst,
> +                                  struct brw_reg payload,
> +                                  struct brw_reg surface,
> +                                  unsigned message_len,
> +                                  unsigned response_len,
> +                                  bool header_present)
> +{
> +   const struct brw_context *brw = p->brw;
> +   struct brw_inst *insn;
> +
> +   if (surface.file != BRW_IMMEDIATE_VALUE) {
> +      struct brw_reg addr = retype(brw_address_reg(0), BRW_REGISTER_TYPE_UD);
> +
> +      brw_push_insn_state(p);
> +      brw_set_default_access_mode(p, BRW_ALIGN_1);
> +      brw_set_default_mask_control(p, BRW_MASK_DISABLE);
> +      brw_set_default_predicate_control(p, BRW_PREDICATE_NONE);
> +
> +      /* Mask out invalid bits from the surface index to avoid hangs e.g. when
> +       * some surface array is accessed out of bounds.
> +       */
> +      insn = brw_AND(p, addr,
> +                     suboffset(vec1(retype(surface, BRW_REGISTER_TYPE_UD)),
> +                               BRW_GET_SWZ(surface.dw1.bits.swizzle, 0)),
> +                     brw_imm_ud(0xff));
> +
> +      brw_pop_insn_state(p);
> +
> +      surface = addr;
> +   }
> +
> +   insn = brw_send_indirect_message(p, sfid, dst, payload, surface);
> +   brw_inst_set_mlen(brw, insn, message_len);
> +   brw_inst_set_rlen(brw, insn, response_len);
> +   brw_inst_set_header_present(brw, insn, header_present);

I'll continue the discussion we started with patch number one here if you
don't mind. What I find confusing is that in case 'surface' is not an
immediate then these three calls modify the OR-instruction. Otherwise they
modify the send. Or am I missing something?