[Mesa-dev] [PATCH 1/6] i965/eu: add support for 1-OWord Block Read/Write messages

Francisco Jerez currojerez at riseup.net
Thu Jun 22 19:10:22 UTC 2017


Samuel Iglesias Gonsálvez <siglesias at igalia.com> writes:

> Signed-off-by: Samuel Iglesias Gonsálvez <siglesias at igalia.com>
> ---
>  src/intel/compiler/brw_eu.h             | 18 ++++++++++------
>  src/intel/compiler/brw_eu_emit.c        | 38 +++++++++++++++++++++++++--------
>  src/intel/compiler/brw_fs_generator.cpp |  5 +++--
>  3 files changed, 43 insertions(+), 18 deletions(-)
>
> diff --git a/src/intel/compiler/brw_eu.h b/src/intel/compiler/brw_eu.h
> index a3a9c63239..723fe2e1b2 100644
> --- a/src/intel/compiler/brw_eu.h
> +++ b/src/intel/compiler/brw_eu.h
> @@ -342,15 +342,19 @@ void brw_oword_block_read(struct brw_codegen *p,
>  unsigned brw_scratch_surface_idx(const struct brw_codegen *p);
>  
>  void brw_oword_block_read_scratch(struct brw_codegen *p,
> -				  struct brw_reg dest,
> -				  struct brw_reg mrf,
> -				  int num_regs,
> -				  unsigned offset);
> +                                  struct brw_reg dest,
> +                                  struct brw_reg mrf,
> +                                  int num_regs,
> +                                  unsigned offset,
> +                                  bool oword1_low,
> +                                  bool oword_high);
>  
>  void brw_oword_block_write_scratch(struct brw_codegen *p,
> -				   struct brw_reg mrf,
> -				   int num_regs,
> -				   unsigned offset);
> +                                   struct brw_reg mrf,
> +                                   int num_regs,
> +                                   unsigned offset,
> +                                   bool oword1_low,
> +                                   bool oword1_high);
>  

This seems like a rather sketchy interface to wrap single-oword scratch
reads and writes, because the num_regs, oword1_low and oword(1?)_high
arguments and the qtr/nib controls specified via p->default all need to
be in close agreement for things not to explode horribly.  Also because
the boolean arguments are highly undescriptive at the call point.

I think a substantially more robust and easy to use interface would be:

|  void brw_oword_block_write_scratch(struct brw_codegen *p,
|                                     struct brw_reg mrf,
|                                     unsigned num_owords,
|                                     unsigned offset);

Where the high/low group controls are specified via p->default,
consistently with other instructions.

>  void gen7_block_read_scratch(struct brw_codegen *p,
>                               struct brw_reg dest,
> diff --git a/src/intel/compiler/brw_eu_emit.c b/src/intel/compiler/brw_eu_emit.c
> index 231d6fdaec..bd6f46c776 100644
> --- a/src/intel/compiler/brw_eu_emit.c
> +++ b/src/intel/compiler/brw_eu_emit.c
> @@ -2133,9 +2133,11 @@ brw_scratch_surface_idx(const struct brw_codegen *p)
>   * register spilling.
>   */
>  void brw_oword_block_write_scratch(struct brw_codegen *p,
> -				   struct brw_reg mrf,
> -				   int num_regs,
> -				   unsigned offset)
> +                                   struct brw_reg mrf,
> +                                   int num_regs,
> +                                   unsigned offset,
> +                                   bool oword1_low,
> +                                   bool oword1_high)
>  {
>     const struct gen_device_info *devinfo = p->devinfo;
>     const unsigned target_cache =
> @@ -2180,6 +2182,14 @@ void brw_oword_block_write_scratch(struct brw_codegen *p,
>        int send_commit_msg;
>        struct brw_reg src_header = retype(brw_vec8_grf(0, 0),
>  					 BRW_REGISTER_TYPE_UW);
> +      int msg_control = BRW_DATAPORT_OWORD_BLOCK_DWORDS(num_regs * 8);
> +
> +      if (num_regs == 1 && (oword1_low || oword1_high)) {
> +         /* Only one of them can be true */
> +         assert(oword1_low ^ oword1_high);
> +         msg_control = oword1_high ?
> +            BRW_DATAPORT_OWORD_BLOCK_1_OWORDHIGH : BRW_DATAPORT_OWORD_BLOCK_1_OWORDLOW;
> +      }
>  
>        brw_inst_set_compression(devinfo, insn, false);
>  
> @@ -2223,7 +2233,7 @@ void brw_oword_block_write_scratch(struct brw_codegen *p,
>        brw_set_dp_write_message(p,
>  			       insn,
>                                 brw_scratch_surface_idx(p),
> -			       BRW_DATAPORT_OWORD_BLOCK_DWORDS(num_regs * 8),
> +			       msg_control,
>  			       msg_type,
>                                 target_cache,
>  			       mlen,
> @@ -2245,10 +2255,12 @@ void brw_oword_block_write_scratch(struct brw_codegen *p,
>   */
>  void
>  brw_oword_block_read_scratch(struct brw_codegen *p,
> -			     struct brw_reg dest,
> -			     struct brw_reg mrf,
> -			     int num_regs,
> -			     unsigned offset)
> +                             struct brw_reg dest,
> +                             struct brw_reg mrf,
> +                             int num_regs,
> +                             unsigned offset,
> +                             bool oword1_low,
> +                             bool oword1_high)
>  {
>     const struct gen_device_info *devinfo = p->devinfo;
>  
> @@ -2291,6 +2303,14 @@ brw_oword_block_read_scratch(struct brw_codegen *p,
>  
>     {
>        brw_inst *insn = next_insn(p, BRW_OPCODE_SEND);
> +      int msg_control = BRW_DATAPORT_OWORD_BLOCK_DWORDS(num_regs * 8);
> +
> +      if (num_regs == 1 && (oword1_low || oword1_high)) {
> +         /* Only one of them can be true */
> +         assert(oword1_low ^ oword1_high);
> +         msg_control = oword1_high ?
> +            BRW_DATAPORT_OWORD_BLOCK_1_OWORDHIGH : BRW_DATAPORT_OWORD_BLOCK_1_OWORDLOW;
> +      }
>  
>        assert(brw_inst_pred_control(devinfo, insn) == 0);
>        brw_inst_set_compression(devinfo, insn, false);
> @@ -2306,7 +2326,7 @@ brw_oword_block_read_scratch(struct brw_codegen *p,
>        brw_set_dp_read_message(p,
>  			      insn,
>                                brw_scratch_surface_idx(p),
> -			      BRW_DATAPORT_OWORD_BLOCK_DWORDS(num_regs * 8),
> +			      msg_control,
>  			      BRW_DATAPORT_READ_MESSAGE_OWORD_BLOCK_READ, /* msg_type */
>  			      target_cache,
>  			      1, /* msg_length */
> diff --git a/src/intel/compiler/brw_fs_generator.cpp b/src/intel/compiler/brw_fs_generator.cpp
> index 2ade486705..32c1a13c16 100644
> --- a/src/intel/compiler/brw_fs_generator.cpp
> +++ b/src/intel/compiler/brw_fs_generator.cpp
> @@ -1179,7 +1179,8 @@ fs_generator::generate_scratch_write(fs_inst *inst, struct brw_reg src)
>  
>        brw_oword_block_write_scratch(p, brw_message_reg(inst->base_mrf),
>                                      block_size,
> -                                    inst->offset + block_size * REG_SIZE * i);
> +                                    inst->offset + block_size * REG_SIZE * i,
> +                                    false, false);
>     }
>  
>     brw_pop_insn_state(p);
> @@ -1192,7 +1193,7 @@ fs_generator::generate_scratch_read(fs_inst *inst, struct brw_reg dst)
>     assert(inst->mlen != 0);
>  
>     brw_oword_block_read_scratch(p, dst, brw_message_reg(inst->base_mrf),
> -                                inst->exec_size / 8, inst->offset);
> +                                inst->exec_size / 8, inst->offset, false, false);
>  }
>  
>  void
> -- 
> 2.11.0
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 212 bytes
Desc: not available
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20170622/de0af6c0/attachment.sig>


More information about the mesa-dev mailing list