On Thu, Jun 14, 2018 at 2:39 PM, Chema Casanova <jmcasan...@igalia.com> wrote:
> On 14/06/18 03:02, Jason Ekstrand wrote: > > On Sat, Jun 9, 2018 at 4:13 AM, Jose Maria Casanova Crespo > > <jmcasan...@igalia.com <mailto:jmcasan...@igalia.com>> wrote: > > > > These new shuffle functions deal with the shuffle/unshuffle > operations > > needed for read/write operations using 32-bit components when the > > read/written components have a different bit-size (8, 16, 64-bits). > > Shuffle from 32-bit to 32-bit becomes a simple MOV. > > > > As the new function shuffle_src_to_dst takes of doing a shuffle or an > > unshuffle based on the different type_sz of source an destination > this > > generic functions work with any source/destination assuming that > writes > > use a 32-bit destination or reads use a 32-bit source. > > > > > > I'm having a lot of trouble understanding this paragraph. Would you > > mind rephrasing it? > > > > Sure, that English didn't compile: > > "shuffle_src_to_dst takes care of doing a shuffle when source type is > smaller than destination type and an unshuffle when source type is > bigger than destination. So this new read/write functions just need > to call shuffle_src_to_dst assuming that writes use a 32-bit > destination and reads use a 32-bit source." > > I included also this comment in the commit log: > > "As shuffle_for_32bit_write/from_32bit_read components take components > in unit of source/destination types and shuffle_src_to_dst takes units > of the smallest type component we adjust the components and > first_component parameters." > Those both look good. > > > > To enable this new functions it is needed than there is no > > source/destination overlap in the case of shuffle_from_32bit_read. > > That never happens on shuffle_for_32bit_write as it allocates a new > > destination register as it was at shuffle_64bit_data_for_32bit_ > write. > > --- > > src/intel/compiler/brw_fs.h | 11 +++++++++ > > src/intel/compiler/brw_fs_nir.cpp | 38 > +++++++++++++++++++++++++++++++ > > 2 files changed, 49 insertions(+) > > > > diff --git a/src/intel/compiler/brw_fs.h > b/src/intel/compiler/brw_fs.h > > index faf51568637..779170ecc95 100644 > > --- a/src/intel/compiler/brw_fs.h > > +++ b/src/intel/compiler/brw_fs.h > > @@ -519,6 +519,17 @@ void shuffle_16bit_data_for_32bit_write(const > > brw::fs_builder &bld, > > const fs_reg &src, > > uint32_t components); > > > > +void shuffle_from_32bit_read(const brw::fs_builder &bld, > > + const fs_reg &dst, > > + const fs_reg &src, > > + uint32_t first_component, > > + uint32_t components); > > + > > +fs_reg shuffle_for_32bit_write(const brw::fs_builder &bld, > > + const fs_reg &src, > > + uint32_t first_component, > > + uint32_t components); > > + > > fs_reg setup_imm_df(const brw::fs_builder &bld, > > double v); > > > > diff --git a/src/intel/compiler/brw_fs_nir.cpp > > b/src/intel/compiler/brw_fs_nir.cpp > > index 1a9d3c41d1d..1f684149fd5 100644 > > --- a/src/intel/compiler/brw_fs_nir.cpp > > +++ b/src/intel/compiler/brw_fs_nir.cpp > > @@ -5454,6 +5454,44 @@ shuffle_src_to_dst(const fs_builder &bld, > > } > > } > > > > +void > > +shuffle_from_32bit_read(const fs_builder &bld, > > + const fs_reg &dst, > > + const fs_reg &src, > > + uint32_t first_component, > > + uint32_t components) > > +{ > > + assert(type_sz(src.type) == 4); > > + > > > > > > /* This function takes components in units of the destination type while > > shuffle_src_to_dst takes components in units of the smallest type */ > > Done. > > > + if (type_sz(dst.type) > 4) { > > + assert(type_sz(dst.type) == 8); > > + first_component *= 2; > > + components *= 2; > > + } > > + > > + shuffle_src_to_dst(bld, dst, src, first_component, components); > > +} > > + > > +fs_reg > > +shuffle_for_32bit_write(const fs_builder &bld, > > + const fs_reg &src, > > + uint32_t first_component, > > + uint32_t components) > > +{ > > + fs_reg dst = bld.vgrf(BRW_REGISTER_TYPE_D, > > + DIV_ROUND_UP (components * > > type_sz(src.type), 4)); > > + > > > > > > /* This function takes components in units of the source type while > > shuffle_src_to_dst takes components in units of the smallest type */ > > Done. > > > With those added and the commit message re-worded a bit, > > > > Reviewed-by: Jason Ekstrand <ja...@jlekstrand.net > > <mailto:ja...@jlekstrand.net>> > > Thanks for the review. > > Chema > > > + if (type_sz(src.type) > 4) { > > + assert(type_sz(src.type) == 8); > > + first_component *= 2; > > + components *= 2; > > + } > > + > > + shuffle_src_to_dst(bld, dst, src, first_component, components); > > + > > + return dst; > > +} > > + > > fs_reg > > setup_imm_df(const fs_builder &bld, double v) > > { > > -- > > 2.17.1 > > > > _______________________________________________ > > 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
