[Mesa-dev] [PATCH 2/7] i965/fs: Support 16-bit do_read_vector with VK_KHR_relaxed_block_layout

[Jason Ekstrand]

On Fri, Feb 23, 2018 at 1:26 AM, Jose Maria Casanova Crespo <
jmcasanova at igalia.com> wrote:

> 16-bit load_ubo/ssbo operations that call do_untyped_read_vector doesn't
> guarantee that offsets are multiple of 4-bytes as required by untyped_read
> message. This happens for example on 16-bit scalar arrays and in the case
> of f16vec3 when then VK_KHR_relaxed_block_layoud is enabled.
>
> Vectors reads when we have non-constant offsets are implemented with
> multiple byte_scattered_read messages that not require 32-bit aligned
> offsets.
> The same applies for constant offsets not aligned to 32-bits.
>
> Untyped_read_surface is used message when there is a constant offset
> 32-bit aligned and we have more than 1 component to read.
> ---
>  src/intel/compiler/brw_fs_nir.cpp | 60 ++++++++++++++++++++++++++++--
> ---------
>  1 file changed, 44 insertions(+), 16 deletions(-)
>
> diff --git a/src/intel/compiler/brw_fs_nir.cpp
> b/src/intel/compiler/brw_fs_nir.cpp
> index 8efec34cc9..45b8e8b637 100644
> --- a/src/intel/compiler/brw_fs_nir.cpp
> +++ b/src/intel/compiler/brw_fs_nir.cpp
> @@ -2305,27 +2305,55 @@ do_untyped_vector_read(const fs_builder &bld,
>     if (type_sz(dest.type) <= 2) {
>        assert(dest.stride == 1);
>
> -      if (num_components > 1) {
> -         /* Pairs of 16-bit components can be read with untyped read, for
> 16-bit
> -          * vec3 4th component is ignored.
> +      unsigned pending_components = num_components;
> +      unsigned first_component = 0;
> +      boolean is_const_offset = offset_reg.file == BRW_IMMEDIATE_VALUE;
> +      fs_reg read_offset;
> +      if (is_const_offset)
> +         read_offset = offset_reg;
> +      else {
> +         read_offset = bld.vgrf(BRW_REGISTER_TYPE_UD);
> +         bld.MOV(read_offset, offset_reg);
> +      }
> +      while (pending_components > 0 &&
> +             (pending_components == 1 ||
> +              !is_const_offset ||
> +              (offset_reg.ud + first_component * 2) % 4)) {
>

I think this would be easier to read as

while (pending_components > 0) {
   if (!is_const_offset || (offset_reg.ud + first_component *
type_sz(dst.type)) % 4) {
      /* Do a single-component load with a byte message */
      first_compoent++;
      pending_components--;
   } else {
      /* Do a load with an untyped read */
      pending_components = 0;
      break;
   }
}

Or, alternatively,

if (is_const_offset) {
   uint32_t start = offset_reg.ud & 3;
   uint32_t end = offset_reg + num_components * type_sz(dst.type);
   end = ALIGN(end, 4);
   assert(end - start <= 16);
   /* Do a load with an untyped read and then throw away unneeded data at
the beginning and the end */
} else {
   /* Do the load using a loop and byte scattered reads */
}

This second one would require a small extension to
shuffle_32bit_load_result_to_16bit_data to handle throwing away data at the
start.


> +         /* Non constant offsets, 16-bit scalars and constant offsets not
> +          * aligned 32-bits are read using one byte_scattered_read message
> +          * for eache  component untyped_read requires 32-bit aligned
> offsets.
>            */
>           fs_reg read_result =
> -            emit_untyped_read(bld, surf_index, offset_reg,
> -                              1 /* dims */, DIV_ROUND_UP(num_components,
> 2),
> -                              BRW_PREDICATE_NONE);
> -         shuffle_32bit_load_result_to_16bit_data(bld,
> -               retype(dest, BRW_REGISTER_TYPE_W),
> -               retype(read_result, BRW_REGISTER_TYPE_D),
> -               num_components);
> -      } else {
> -         assert(num_components == 1);
> -         /* scalar 16-bit are read using one byte_scattered_read message
> */
> -         fs_reg read_result =
> -            emit_byte_scattered_read(bld, surf_index, offset_reg,
> +            emit_byte_scattered_read(bld, surf_index, read_offset,
>                                       1 /* dims */, 1,
>                                       type_sz(dest.type) * 8 /* bit_size
> */,
>                                       BRW_PREDICATE_NONE);
> -         bld.MOV(dest, subscript(read_result, dest.type, 0));
> +         shuffle_32bit_load_result_to_16bit_data(bld,
> +               retype(offset(dest, bld, first_component),
> BRW_REGISTER_TYPE_W),
> +               retype(read_result, BRW_REGISTER_TYPE_D),
> +               1);
> +         pending_components--;
> +         first_component ++;
> +         if (is_const_offset)
> +            read_offset.ud += 2;
> +         else
> +            bld.ADD(read_offset, offset_reg, brw_imm_ud(2 *
> first_component));
> +      }
> +      assert(pending_components != 1);
> +      if (pending_components > 1) {
> +         assert (is_const_offset &&
> +                 (offset_reg.ud + first_component * 2) % 4 == 0);
> +         /* At this point we have multiple 16-bit components that have
> constant
> +          * offset multiple of 4-bytes that can be read with
> untyped_reads.
> +          */
> +         fs_reg read_result =
> +            emit_untyped_read(bld, surf_index, read_offset,
> +                              1 /* dims */, DIV_ROUND_UP(pending_components,
> 2),
> +                              BRW_PREDICATE_NONE);
> +         shuffle_32bit_load_result_to_16bit_data(bld,
> +               retype(offset(dest,bld,first_component),
> BRW_REGISTER_TYPE_W),
> +               retype(read_result, BRW_REGISTER_TYPE_D),
> +               pending_components);
>        }
>     } else if (type_sz(dest.type) == 4) {
>        fs_reg read_result = emit_untyped_read(bld, surf_index, offset_reg,
> --
> 2.14.3
>
> _______________________________________________
> 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/20180223/efd1a3e6/attachment-0001.html>