On Thu, 2016-05-12 at 20:05 -0700, Francisco Jerez wrote: > Samuel Iglesias Gonsálvez <sigles...@igalia.com> writes: > > > From: Iago Toral Quiroga <ito...@igalia.com> > > > > This does the inverse operation of shuffle_32bit_load_result_to_64bit_data > > and we will use it when we need to write 64-bit data in the layout expected > > by untyped write messages. > > > > v2 (curro): > > - Use subscript() instead of stride() > > - Assert on the input types rather than silently retyping. > > - Use offset() instead of horiz_offset(), drop the multiplier definition. > > - Drop the temporary vgrf and force_writemask_all. > > - Make component_i const. > > --- > > src/mesa/drivers/dri/i965/brw_fs.cpp | 40 > > ++++++++++++++++++++++++++++++++++++ > > src/mesa/drivers/dri/i965/brw_fs.h | 5 +++++ > > 2 files changed, 45 insertions(+) > > > > diff --git a/src/mesa/drivers/dri/i965/brw_fs.cpp > > b/src/mesa/drivers/dri/i965/brw_fs.cpp > > index ebc5128..3c58ccb 100644 > > --- a/src/mesa/drivers/dri/i965/brw_fs.cpp > > +++ b/src/mesa/drivers/dri/i965/brw_fs.cpp > > @@ -280,6 +280,46 @@ > > fs_visitor::shuffle_32bit_load_result_to_64bit_data(const fs_builder &bld, > > } > > > > /** > > + * This helper does the inverse operation of > > + * SHUFFLE_32BIT_LOAD_RESULT_TO_64BIT_DATA. > > + * > > + * We need to do this when we are going to use untyped write messsages that > > + * operate with 32-bit components in order to arrange our 64-bit data to be > > + * in the expected layout. > > + */ > > +void > > +fs_visitor::shuffle_64bit_data_for_32bit_write(const fs_builder &bld, > > + const fs_reg dst, > > + const fs_reg src, > > + uint32_t components) > > +{ > > + assert(type_sz(src.type) == 8); > > + assert(type_sz(dst.type) == 4); > > + > > + /* A temporary that we will use to shuffle the 64-bit data of each > > + * component in the vector into valid 32-bit data that we can write. > > + * We can't write directly to dst because dst can be (and would usually > > be) > > + * the same as src and in that case the first MOV in the loop below > > would > > + * overwrite the data read in the second MOV. > > + */ > > + fs_reg tmp = retype(bld.vgrf(src.type), dst.type); > > +
For the record, I have been thinking that unlike the case of the other shuffling function that we use when we need to shuffle the data of a read, it should usually be unsafe to call this one with src == dst (and in fact our code doesn't) because the original data representing valid 64-bit elements might still be used after the write but it would no longer represent valid 64-bit data elements if we do the shuffling in-place, so in this case I think we can remove the temporary and write to the dst directly as you originally suggested. I'll do that change and if I don't see anything wrong with it I'll send a v2. > This is just 'bld.vgrf(dst.type, 2)' as I suggested off-list, other than > that: > > Reviewed-by: Francisco Jerez <curroje...@riseup.net> > > > + for (unsigned i = 0; i < components; i++) { > > + const fs_reg component_i = offset(src, bld, i); > > + > > + bld.MOV(tmp, subscript(component_i, dst.type, 0)); > > + bld.MOV(offset(tmp, bld, 1), subscript(component_i, dst.type, 1)); > > + > > + /* Because we are shuffling our 64-bit data to a 32-bit layout, we > > + * need these MOVs to be 32-bit (instead of doing this as a single > > + * DF MOV), otherwise we would not be respecting the channel enables. > > + */ > > + bld.MOV(offset(dst, bld, 2 * i), tmp); > > + bld.MOV(offset(dst, bld, 2 * i + 1), offset(tmp, bld, 1)); > > + } > > +} > > + > > +/** > > * A helper for MOV generation for fixing up broken hardware SEND > > dependency > > * handling. > > */ > > diff --git a/src/mesa/drivers/dri/i965/brw_fs.h > > b/src/mesa/drivers/dri/i965/brw_fs.h > > index 1aeacae..4c17ff4 100644 > > --- a/src/mesa/drivers/dri/i965/brw_fs.h > > +++ b/src/mesa/drivers/dri/i965/brw_fs.h > > @@ -110,6 +110,11 @@ public: > > const fs_reg src, > > uint32_t components); > > > > + void shuffle_64bit_data_for_32bit_write(const brw::fs_builder &bld, > > + const fs_reg dst, > > + const fs_reg src, > > + uint32_t components); > > + > > void do_untyped_vector_read(const brw::fs_builder &bld, > > const fs_reg surf_index, > > const fs_reg offset_reg, > > -- > > 2.5.0 > > > > _______________________________________________ > > 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 _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev