On Mon, Dec 4, 2017 at 4:50 PM, Chema Casanova <jmcasan...@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 > > <jmcasan...@igalia.com <mailto:jmcasan...@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@lists.freedesktop.org <mailto:mesa-dev@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@lists.freedesktop.org > > https://lists.freedesktop.org/mailman/listinfo/mesa-dev > > >
_______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
