[Mesa-dev] [PATCH 07/14] intel/compiler: shuffle_from_32bit_read for 64-bit do_untyped_vector_read

[Chema Casanova]

On 14/06/18 03:26, 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:
> 
>     do_untyped_vector_read is used at load_ssbo and load_shared.
> 
>     The previous MOVs are removed because shuffle_from_32bit_read
>     can handle storing the shuffle results in the expected destination
>     just using the proper offset.
>     ---
>      src/intel/compiler/brw_fs_nir.cpp | 12 ++----------
>      1 file changed, 2 insertions(+), 10 deletions(-)
> 
>     diff --git a/src/intel/compiler/brw_fs_nir.cpp
>     b/src/intel/compiler/brw_fs_nir.cpp
>     index 7e738ade82e..780a9e228de 100644
>     --- a/src/intel/compiler/brw_fs_nir.cpp
>     +++ b/src/intel/compiler/brw_fs_nir.cpp
>     @@ -2434,16 +2434,8 @@ do_untyped_vector_read(const fs_builder &bld,
>                                                      BRW_PREDICATE_NONE);
> 
>               /* Shuffle the 32-bit load result into valid 64-bit data */
>     -         const fs_reg packed_result = bld.vgrf(dest.type,
>     iter_components);
>     -         shuffle_32bit_load_result_to_64bit_data(
>     -            bld, packed_result, read_result, iter_components);
>     -
>     -         /* Move each component to its destination */
>     -         read_result = retype(read_result, BRW_REGISTER_TYPE_DF);
>     -         for (int c = 0; c < iter_components; c++) {
>     -            bld.MOV(offset(dest, bld, it * 2 + c),
>     -                    offset(packed_result, bld, c));
>     -         }
> 
> 
> I really don't know why we needed this extra set of MOVs.  They seem
> pretty pointless to me.  Maybe history?  In any case, this looks good.v-

I've just checked and there is not much history as the 64-bit code of
this function hasn't been changed since they landed. I think that the
logic was first shuffle and then move to the proper destination instead
of just shuffling to the final destination directly.

So maybe Iago remembers if there was any reason why...

> Reviewed-by: Jason Ekstrand <jason at jlekstrand.net
> <mailto:jason at jlekstrand.net>>
>  
> 
>     +         shuffle_from_32bit_read(bld, offset(dest, bld, it * 2),
>     +                                 read_result, 0, iter_components);
> 
>               bld.ADD(read_offset, read_offset, brw_imm_ud(16));
>            }
>     -- 
>     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
>