[Mesa-dev] [PATCH v3 22/43] i965/fs: Use byte_scattered_write on 16-bit store_ssbo

[Jason Ekstrand]

On Thu, Oct 12, 2017 at 11:38 AM, Jose Maria Casanova Crespo <
jmcasanova at igalia.com> wrote:

> From: Alejandro Piñeiro <apinheiro at igalia.com>
>
> We need to rely on byte scattered writes as untyped writes are 32-bit
> size. We could try to keep using 32-bit messages when we have two or
> four 16-bit elements, but for simplicity sake, we use the same message
> for any component number. We revisit this in this serie.
>
> v2: Removed use of stride = 2 on 16-bit sources (Jason Ekstrand)
>
> Signed-off-by: Jose Maria Casanova Crespo <jmcasanova at igalia.com>
> Signed-off-by: Alejandro Piñeiro <apinheiro at igalia.com
> ---
>  src/intel/compiler/brw_fs_nir.cpp | 35 +++++++++++++++++++++++++++++-
> -----
>  1 file changed, 29 insertions(+), 6 deletions(-)
>
> diff --git a/src/intel/compiler/brw_fs_nir.cpp
> b/src/intel/compiler/brw_fs_nir.cpp
> index e108b5517b..13c16fc912 100644
> --- a/src/intel/compiler/brw_fs_nir.cpp
> +++ b/src/intel/compiler/brw_fs_nir.cpp
> @@ -28,6 +28,7 @@
>
>  using namespace brw;
>  using namespace brw::surface_access;
> +using namespace brw::scattered_access;
>
>  void
>  fs_visitor::emit_nir_code()
> @@ -4085,8 +4086,15 @@ fs_visitor::nir_emit_intrinsic(const fs_builder
> &bld, nir_intrinsic_instr *instr
>            * length of the write to what we can do and let the next
> iteration
>            * handle the rest
>            */
> -         if (type_size > 4)
> +         if (type_size > 4) {
>

Maybe move the above comment to here.


>              length = MIN2(2, length);
> +         } else if (type_size == 2) {
> +            /* For 16-bit types we are using byte scattered writes, that
> can
> +             * only write one component per call. So we limit the length,
> and
> +             * let the write happening in several iterations.
> +             */
> +            length = 1;
> +         }
>
>           fs_reg offset_reg;
>           nir_const_value *const_offset = nir_src_as_const_value(instr->
> src[2]);
> @@ -4100,11 +4108,26 @@ fs_visitor::nir_emit_intrinsic(const fs_builder
> &bld, nir_intrinsic_instr *instr
>                      brw_imm_ud(type_size * first_component));
>           }
>
> -
> -         emit_untyped_write(bld, surf_index, offset_reg,
> -                            offset(val_reg, bld, first_component *
> type_slots),
> -                            1 /* dims */, length * type_slots,
> -                            BRW_PREDICATE_NONE);
> +         if (type_size == 2) {
> +            /* Untyped Surface messages have a fixed 32-bit size, so we
> need
> +             * to rely on byte scattered in order to write 16-bit
> elements.
> +             * The byte_scattered_write message needs that every written
> 16-bit
> +             * type to be aligned 32-bits (stride=2).
> +             */
> +            fs_reg tmp = bld.vgrf(BRW_REGISTER_TYPE_F);
> +            val_reg.type = BRW_REGISTER_TYPE_HF;
> +            bld.MOV (subscript(tmp, BRW_REGISTER_TYPE_HF, 0),
> +                     offset(val_reg, bld, first_component));
>

We should probably use W types here so we don't get any float conversion
problems.

With that fixed and the previous patch squashed in,

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


> +            emit_byte_scattered_write(bld, surf_index, offset_reg,
> +                                      tmp,
> +                                      1 /* dims */, length * type_slots,
> +                                      BRW_PREDICATE_NONE);
> +         } else {
> +            emit_untyped_write(bld, surf_index, offset_reg,
> +                               offset(val_reg, bld, first_component *
> type_slots),
> +                               1 /* dims */, length * type_slots,
> +                               BRW_PREDICATE_NONE);
> +         }
>
>           /* Clear the bits in the writemask that we just wrote, then try
>            * again to see if more channels are left.
> --
> 2.13.6
>
> _______________________________________________
> 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/20171030/eba78e70/attachment.html>