[Mesa-dev] [PATCH v2 2/8] i965/fs: shuffle_32bit_load_result_to_16bit_data now skips components

[Chema Casanova]

On 27/02/18 20:00, Jason Ekstrand wrote:
> On Tue, Feb 27, 2018 at 5:27 AM, Jose Maria Casanova Crespo
> <jmcasanova at igalia.com <mailto:jmcasanova at igalia.com>> wrote:
> 
>     This helper used to load 16bit components from 32-bits read now allows
>     skipping components with the new parameter first_component. The
>     semantics
>     now skip components until we reach the first_component, and then
>     reads the
>     number of components passed to the function.
> 
>     All previous uses of the helper are updated to use 0 as first_component.
>     This will allow read 16-bit components when the first one is not aligned
>     32-bit. Enabling more usages of untyped_reads with 16-bit types.
>     ---
>      src/intel/compiler/brw_fs.cpp     | 2 +-
>      src/intel/compiler/brw_fs.h       | 3 ++-
>      src/intel/compiler/brw_fs_nir.cpp | 8 +++++---
>      3 files changed, 8 insertions(+), 5 deletions(-)
> 
>     diff --git a/src/intel/compiler/brw_fs.cpp
>     b/src/intel/compiler/brw_fs.cpp
>     index bed632d21b9..e961b76ab61 100644
>     --- a/src/intel/compiler/brw_fs.cpp
>     +++ b/src/intel/compiler/brw_fs.cpp
>     @@ -194,7 +194,7 @@ fs_visitor::VARYING_PULL_CONSTANT_LOAD(const
>     fs_builder &bld,
>         fs_reg dw = offset(vec4_result, bld, (const_offset & 0xf) / 4);
>         switch (type_sz(dst.type)) {
>         case 2:
>     -      shuffle_32bit_load_result_to_16bit_data(bld, dst, dw, 1);
>     +      shuffle_32bit_load_result_to_16bit_data(bld, dst, dw, 1, 0);
>            bld.MOV(dst, subscript(dw, dst.type, (const_offset / 2) & 1));
>            break;
>         case 4:
>     diff --git a/src/intel/compiler/brw_fs.h b/src/intel/compiler/brw_fs.h
>     index 63373580ee4..52dd5e1d6bb 100644
>     --- a/src/intel/compiler/brw_fs.h
>     +++ b/src/intel/compiler/brw_fs.h
>     @@ -503,7 +503,8 @@ fs_reg shuffle_64bit_data_for_32bit_write(const
>     brw::fs_builder &bld,
>      void shuffle_32bit_load_result_to_16bit_data(const brw::fs_builder
>     &bld,
>                                                   const fs_reg &dst,
>                                                   const fs_reg &src,
>     -                                             uint32_t components);
>     +                                             uint32_t components,
>     +                                             uint32_t first_component);
> 
> 
> I hope this doesn't cause too much trouble, but I think it would be
> better to have first_component come before components in the argument
> list.  With that changed,

Not too much, it only affected 2 patches in the input/output series that
were already affected by this patch. So everything already changed locally.

> Reviewed-by: Jason Ekstrand <jason at jlekstrand.net
> <mailto:jason at jlekstrand.net>>

Thanks,

Chema

>      void shuffle_16bit_data_for_32bit_write(const brw::fs_builder &bld,
>                                              const fs_reg &dst,
>     diff --git a/src/intel/compiler/brw_fs_nir.cpp
>     b/src/intel/compiler/brw_fs_nir.cpp
>     index 4aa411d149f..5567433a19e 100644
>     --- a/src/intel/compiler/brw_fs_nir.cpp
>     +++ b/src/intel/compiler/brw_fs_nir.cpp
>     @@ -2316,7 +2316,7 @@ do_untyped_vector_read(const fs_builder &bld,
>               shuffle_32bit_load_result_to_16bit_data(bld,
>                     retype(dest, BRW_REGISTER_TYPE_W),
>                     retype(read_result, BRW_REGISTER_TYPE_D),
>     -               num_components);
>     +               num_components, 0);
>            } else {
>               assert(num_components == 1);
>               /* scalar 16-bit are read using one byte_scattered_read
>     message */
>     @@ -4908,7 +4908,8 @@ void
>      shuffle_32bit_load_result_to_16bit_data(const fs_builder &bld,
>                                              const fs_reg &dst,
>                                              const fs_reg &src,
>     -                                        uint32_t components)
>     +                                        uint32_t components,
>     +                                        uint32_t first_component)
>      {
>         assert(type_sz(src.type) == 4);
>         assert(type_sz(dst.type) == 2);
>     @@ -4922,7 +4923,8 @@ shuffle_32bit_load_result_to_16bit_data(const
>     fs_builder &bld,
> 
>         for (unsigned i = 0; i < components; i++) {
>            const fs_reg component_i =
>     -         subscript(offset(src, bld, i / 2), dst.type, i % 2);
>     +         subscript(offset(src, bld, (first_component + i) / 2),
>     dst.type,
>     +                   (first_component + i) % 2);
> 
>            bld.MOV(offset(tmp, bld, i % 2), component_i);
> 
>     --
>     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>
> 
>