[Mesa-dev] [PATCH v4 20/44] i965/fs: Add byte scattered read message and fs support
Jason Ekstrand
jason at jlekstrand.net
Tue Dec 5 01:22:28 UTC 2017
On Mon, Dec 4, 2017 at 4:50 PM, Chema Casanova <jmcasanova at igalia.com>
wrote:
> On 30/11/17 21:45, Jason Ekstrand wrote:
> > On Wed, Nov 29, 2017 at 6:50 PM, Jose Maria Casanova Crespo
> > <jmcasanova at igalia.com <mailto:jmcasanova at igalia.com>> wrote:
> >
> > v2: Fix alignment style (Topi Pohjolainen)
> > (Jason Ekstrand)
> > - Enable bit_size parameter to scattered messages to enable
> > different
> > bitsizes byte/word/dword.
> > - Remove use of brw_send_indirect_scattered_message in favor of
> > brw_send_indirect_surface_message.
> > - Move scattered messages to surface messages namespace.
> > - Assert align1 for scattered messages and assume Gen8+.
> > - Inline brw_set_dp_byte_scattered_read.
> > ---
> > src/intel/compiler/brw_eu.h | 8 +++++++
> > src/intel/compiler/brw_eu_defines.h | 2 ++
> > src/intel/compiler/brw_eu_emit.c | 30
> > ++++++++++++++++++++++++++
> > src/intel/compiler/brw_fs.cpp | 19
> ++++++++++++++++
> > src/intel/compiler/brw_fs_copy_propagation.cpp | 2 ++
> > src/intel/compiler/brw_fs_generator.cpp | 6 ++++++
> > src/intel/compiler/brw_fs_surface_builder.cpp | 11 +++++++++-
> > src/intel/compiler/brw_fs_surface_builder.h | 7 ++++++
> > src/intel/compiler/brw_shader.cpp | 6 ++++++
> > 9 files changed, 90 insertions(+), 1 deletion(-)
> >
> > diff --git a/src/intel/compiler/brw_eu.h
> b/src/intel/compiler/brw_eu.h
> > index 3ac3b4342a..2d0f56f793 100644
> > --- a/src/intel/compiler/brw_eu.h
> > +++ b/src/intel/compiler/brw_eu.h
> > @@ -485,6 +485,14 @@ brw_typed_surface_write(struct brw_codegen *p,
> > unsigned msg_length,
> > unsigned num_channels);
> >
> > +void
> > +brw_byte_scattered_read(struct brw_codegen *p,
> > + struct brw_reg dst,
> > + struct brw_reg payload,
> > + struct brw_reg surface,
> > + unsigned msg_length,
> > + unsigned bit_size);
> > +
> > void
> > brw_byte_scattered_write(struct brw_codegen *p,
> > struct brw_reg payload,
> > diff --git a/src/intel/compiler/brw_eu_defines.h
> > b/src/intel/compiler/brw_eu_defines.h
> > index de6330ee54..aa510ebfa4 100644
> > --- a/src/intel/compiler/brw_eu_defines.h
> > +++ b/src/intel/compiler/brw_eu_defines.h
> > @@ -409,6 +409,8 @@ enum opcode {
> > * opcode, but instead of taking a single payload blog they
> > expect their
> > * arguments separately as individual sources, like untyped
> > write/read.
> > */
> > + SHADER_OPCODE_BYTE_SCATTERED_READ,
> > + SHADER_OPCODE_BYTE_SCATTERED_READ_LOGICAL,
> > SHADER_OPCODE_BYTE_SCATTERED_WRITE,
> > SHADER_OPCODE_BYTE_SCATTERED_WRITE_LOGICAL,
> >
> > diff --git a/src/intel/compiler/brw_eu_emit.c
> > b/src/intel/compiler/brw_eu_emit.c
> > index ded7e228cf..bdc516848a 100644
> > --- a/src/intel/compiler/brw_eu_emit.c
> > +++ b/src/intel/compiler/brw_eu_emit.c
> > @@ -2998,6 +2998,36 @@ static enum brw_data_size
> > brw_data_size_from_bit_size(unsigned bit_size)
> > }
> > }
> >
> > +
> > +void
> > +brw_byte_scattered_read(struct brw_codegen *p,
> > + struct brw_reg dst,
> > + struct brw_reg payload,
> > + struct brw_reg surface,
> > + unsigned msg_length,
> > + unsigned bit_size)
> > +{
> > + assert(brw_inst_access_mode(p->devinfo, p->current) ==
> BRW_ALIGN_1);
> > + const struct gen_device_info *devinfo = p->devinfo;
> > + const unsigned sfid = GEN7_SFID_DATAPORT_DATA_CACHE;
> > +
> > + struct brw_inst *insn = brw_send_indirect_surface_message(
> > + p, sfid, dst, payload, surface, msg_length,
> > + brw_surface_payload_size(p, 1, true, true),
> > + false);
> > +
> > + unsigned msg_control = brw_data_size_from_bit_size(bit_size) <<
> 2;
> > +
> > + if (brw_inst_exec_size(devinfo, p->current) == BRW_EXECUTE_16)
> > + msg_control |= 1; /* SIMD16 mode */
> > + else
> > + msg_control |= 0; /* SIMD8 mode */
> > +
> > + brw_inst_set_dp_msg_type(devinfo, insn,
> > + HSW_DATAPORT_DC_PORT0_BYTE_
> SCATTERED_READ);
> > + brw_inst_set_dp_msg_control(devinfo, insn, msg_control);
> > +}
> > +
> > void
> > brw_byte_scattered_write(struct brw_codegen *p,
> > struct brw_reg payload,
> > diff --git a/src/intel/compiler/brw_fs.cpp
> > b/src/intel/compiler/brw_fs.cpp
> > index 32f1d757f0..1ca4d416b2 100644
> > --- a/src/intel/compiler/brw_fs.cpp
> > +++ b/src/intel/compiler/brw_fs.cpp
> > @@ -251,6 +251,7 @@ fs_inst::is_send_from_grf() const
> > case SHADER_OPCODE_UNTYPED_SURFACE_READ:
> > case SHADER_OPCODE_UNTYPED_SURFACE_WRITE:
> > case SHADER_OPCODE_BYTE_SCATTERED_WRITE:
> > + case SHADER_OPCODE_BYTE_SCATTERED_READ:
> > case SHADER_OPCODE_TYPED_ATOMIC:
> > case SHADER_OPCODE_TYPED_SURFACE_READ:
> > case SHADER_OPCODE_TYPED_SURFACE_WRITE:
> > @@ -750,6 +751,16 @@ fs_inst::components_read(unsigned i) const
> > else
> > return 1;
> >
> > + case SHADER_OPCODE_BYTE_SCATTERED_READ_LOGICAL:
> > + assert(src[3].file == IMM &&
> > + src[4].file == IMM);
> > + if (i == 0)
> > + return 1;
> > + else if (i == 1)
> > + return 0;
> >
> >
> > I suppose src[1] is for the data to write which is unused for this
> > opcode? If so, I'd rather we just do
> >
> > /* src[1] is normally for data to write and is unused */
> > return i == 1 ? 0 : 1;
>
> Yes, that is the reason, in this case as it is a read operation, it
> isn't using the source at the payload, the only special change with '
> other surfaces ops is that we are using src[4] as IMM passing the
> bitsize for the scattered read, and dimension should always be 1, so the
> source would be always 1 component for write operations.
>
> Would it make sense to include a comment with the explanation of the
> params to explain why we use src[4]?
>
> Something like:
>
> /* Scattered logical opcodes use the following params:
> * src[0] Surface coordinates
> * src[1] Surface operation source (ignored for reads)
> * src[2] Surface
> * src[3] IMM with always 1 dimension.
> * src[4] IMM with arg bitsize for scattered read/write 8, 16, 32 */
>
That would be fine with me.
With your change, Can I consider this patch as R-b?
>
Sure.
> > + else
> > + return 1;
> > +
> > case SHADER_OPCODE_BYTE_SCATTERED_WRITE_LOGICAL:
> > assert(src[3].file == IMM &&
> > src[4].file == IMM);
> > @@ -798,6 +809,7 @@ fs_inst::size_read(int arg) const
> > case SHADER_OPCODE_TYPED_SURFACE_WRITE:
> > case FS_OPCODE_INTERPOLATE_AT_PER_SLOT_OFFSET:
> > case SHADER_OPCODE_BYTE_SCATTERED_WRITE:
> > + case SHADER_OPCODE_BYTE_SCATTERED_READ:
> > if (arg == 0)
> > return mlen * REG_SIZE;
> > break;
> > @@ -4545,6 +4557,12 @@ fs_visitor::lower_logical_sends()
> > ibld.sample_mask_reg());
> > break;
> >
> > + case SHADER_OPCODE_BYTE_SCATTERED_READ_LOGICAL:
> > + lower_surface_logical_send(ibld, inst,
> > + SHADER_OPCODE_BYTE_SCATTERED_
> READ,
> > + fs_reg());
> > + break;
> > +
> > case SHADER_OPCODE_BYTE_SCATTERED_WRITE_LOGICAL:
> > lower_surface_logical_send(ibld, inst,
> > SHADER_OPCODE_BYTE_SCATTERED_
> WRITE,
> > @@ -5036,6 +5054,7 @@ get_lowered_simd_width(const struct
> > gen_device_info *devinfo,
> > case SHADER_OPCODE_UNTYPED_SURFACE_READ_LOGICAL:
> > case SHADER_OPCODE_UNTYPED_SURFACE_WRITE_LOGICAL:
> > case SHADER_OPCODE_BYTE_SCATTERED_WRITE_LOGICAL:
> > + case SHADER_OPCODE_BYTE_SCATTERED_READ_LOGICAL:
> > return MIN2(16, inst->exec_size);
> >
> > case SHADER_OPCODE_URB_READ_SIMD8:
> > diff --git a/src/intel/compiler/brw_fs_copy_propagation.cpp
> > b/src/intel/compiler/brw_fs_copy_propagation.cpp
> > index fcf4706b7a..d4d01d783c 100644
> > --- a/src/intel/compiler/brw_fs_copy_propagation.cpp
> > +++ b/src/intel/compiler/brw_fs_copy_propagation.cpp
> > @@ -656,6 +656,7 @@ fs_visitor::try_constant_propagate(fs_inst
> > *inst, acp_entry *entry)
> > case SHADER_OPCODE_TYPED_SURFACE_READ:
> > case SHADER_OPCODE_TYPED_SURFACE_WRITE:
> > case SHADER_OPCODE_BYTE_SCATTERED_WRITE:
> > + case SHADER_OPCODE_BYTE_SCATTERED_READ:
> > /* We only propagate into the surface argument of the
> > * instruction. Everything else goes through LOAD_PAYLOAD.
> > */
> > @@ -696,6 +697,7 @@ fs_visitor::try_constant_propagate(fs_inst
> > *inst, acp_entry *entry)
> > case SHADER_OPCODE_TYPED_SURFACE_READ_LOGICAL:
> > case SHADER_OPCODE_TYPED_SURFACE_WRITE_LOGICAL:
> > case SHADER_OPCODE_BYTE_SCATTERED_WRITE_LOGICAL:
> > + case SHADER_OPCODE_BYTE_SCATTERED_READ_LOGICAL:
> > inst->src[i] = val;
> > progress = true;
> > break;
> > diff --git a/src/intel/compiler/brw_fs_generator.cpp
> > b/src/intel/compiler/brw_fs_generator.cpp
> > index fedc9acf97..a3861cd68e 100644
> > --- a/src/intel/compiler/brw_fs_generator.cpp
> > +++ b/src/intel/compiler/brw_fs_generator.cpp
> > @@ -2073,6 +2073,12 @@ fs_generator::generate_code(const cfg_t *cfg,
> > int dispatch_width)
> > inst->mlen, src[2].ud);
> > break;
> >
> > + case SHADER_OPCODE_BYTE_SCATTERED_READ:
> > + assert(src[2].file == BRW_IMMEDIATE_VALUE);
> > + brw_byte_scattered_read(p, dst, src[0], src[1],
> > + inst->mlen, src[2].ud);
> > + break;
> > +
> > case SHADER_OPCODE_BYTE_SCATTERED_WRITE:
> > assert(src[2].file == BRW_IMMEDIATE_VALUE);
> > brw_byte_scattered_write(p, src[0], src[1],
> > diff --git a/src/intel/compiler/brw_fs_surface_builder.cpp
> > b/src/intel/compiler/brw_fs_surface_builder.cpp
> > index 37cc29e361..c346ef9e70 100644
> > --- a/src/intel/compiler/brw_fs_surface_builder.cpp
> > +++ b/src/intel/compiler/brw_fs_surface_builder.cpp
> > @@ -161,6 +161,16 @@ namespace brw {
> > addr, tmp, surface, dims, op, rsize);
> > }
> >
> > + fs_reg
> > + emit_byte_scattered_read(const fs_builder &bld,
> > + const fs_reg &surface, const fs_reg
> > &addr,
> > + unsigned dims, unsigned size,
> > + unsigned bit_size, brw_predicate
> pred)
> > + {
> > + return emit_send(bld,
> > SHADER_OPCODE_BYTE_SCATTERED_READ_LOGICAL,
> > + addr, fs_reg(), surface, dims, bit_size,
> > size, pred);
> > + }
> > +
> > void
> > emit_byte_scattered_write(const fs_builder &bld, const fs_reg
> > &surface,
> > const fs_reg &addr, const fs_reg
> &src,
> > @@ -1202,4 +1212,3 @@ namespace brw {
> > }
> > }
> > }
> > -
> > diff --git a/src/intel/compiler/brw_fs_surface_builder.h
> > b/src/intel/compiler/brw_fs_surface_builder.h
> > index bf9a8c68c8..194d61d489 100644
> > --- a/src/intel/compiler/brw_fs_surface_builder.h
> > +++ b/src/intel/compiler/brw_fs_surface_builder.h
> > @@ -64,6 +64,13 @@ namespace brw {
> > unsigned dims, unsigned rsize, unsigned op,
> > brw_predicate pred = BRW_PREDICATE_NONE);
> >
> > + fs_reg
> > + emit_byte_scattered_read(const fs_builder &bld,
> > + const fs_reg &surface, const fs_reg
> > &addr,
> > + unsigned dims, unsigned size,
> > + unsigned bit_size,
> > + brw_predicate pred =
> > BRW_PREDICATE_NONE);
> > +
> > void
> > emit_byte_scattered_write(const fs_builder &bld, const fs_reg
> > &surface,
> > const fs_reg &addr, const fs_reg
> &src,
> > diff --git a/src/intel/compiler/brw_shader.cpp
> > b/src/intel/compiler/brw_shader.cpp
> > index 209552e1b2..74b52976d7 100644
> > --- a/src/intel/compiler/brw_shader.cpp
> > +++ b/src/intel/compiler/brw_shader.cpp
> > @@ -293,6 +293,10 @@ brw_instruction_name(const struct
> > gen_device_info *devinfo, enum opcode op)
> > case SHADER_OPCODE_MEMORY_FENCE:
> > return "memory_fence";
> >
> > + case SHADER_OPCODE_BYTE_SCATTERED_READ:
> > + return "byte_scattered_read";
> > + case SHADER_OPCODE_BYTE_SCATTERED_READ_LOGICAL:
> > + return "byte_scattered_read_logical";
> > case SHADER_OPCODE_BYTE_SCATTERED_WRITE:
> > return "byte_scattered_write";
> > case SHADER_OPCODE_BYTE_SCATTERED_WRITE_LOGICAL:
> > @@ -999,6 +1003,8 @@ backend_instruction::is_volatile() const
> > case SHADER_OPCODE_UNTYPED_SURFACE_READ_LOGICAL:
> > case SHADER_OPCODE_TYPED_SURFACE_READ:
> > case SHADER_OPCODE_TYPED_SURFACE_READ_LOGICAL:
> > + case SHADER_OPCODE_BYTE_SCATTERED_READ:
> > + case SHADER_OPCODE_BYTE_SCATTERED_READ_LOGICAL:
> > case SHADER_OPCODE_URB_READ_SIMD8:
> > case SHADER_OPCODE_URB_READ_SIMD8_PER_SLOT:
> > case VEC4_OPCODE_URB_READ:
> > --
> > 2.14.3
> >
> > _______________________________________________
> > mesa-dev mailing list
> > mesa-dev at lists.freedesktop.org <mailto:mesa-dev at lists.
> freedesktop.org>
> > https://lists.freedesktop.org/mailman/listinfo/mesa-dev
> > <https://lists.freedesktop.org/mailman/listinfo/mesa-dev>
> >
> >
> >
> >
> > _______________________________________________
> > mesa-dev mailing list
> > mesa-dev at lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/mesa-dev
> >
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20171204/2aec8915/attachment-0001.html>
More information about the mesa-dev
mailing list