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

[Francisco Jerez]

"Pohjolainen, Topi" <topi.pohjolainen at intel.com> writes:

> 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?

Yeah, that's the whole point of the OR instruction, indirect message
sends no longer have an immediate source so all these control bits have
to be specified somewhere else.  The caller doesn't care whether the
returned instruction is a SEND or some other opcode as long as it has
room for the control fields.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 212 bytes
Desc: not available
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20150306/3c7480c0/attachment.sig>