[Mesa-dev] [PATCH 02/14] intel/compiler: new shuffle_for_32bit_write and shuffle_from_32bit_read

Jason Ekstrand jason at jlekstrand.net
Thu Jun 14 23:56:27 UTC 2018


On Thu, Jun 14, 2018 at 2:39 PM, Chema Casanova <jmcasanova at 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
> > <jmcasanova at igalia.com <mailto:jmcasanova at 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 <jason at jlekstrand.net
> > <mailto:jason at 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 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/20180614/517b30c8/attachment-0001.html>


More information about the mesa-dev mailing list