[Mesa-dev] [PATCH v3] i965/fs: indirect addressing with doubles is not supported in CHV/BSW/BXT

[Samuel Iglesias Gonsálvez]

On 17/06/16 10:43, Samuel Iglesias Gonsálvez wrote:
> From the Cherryview's PRM, Volume 7, 3D Media GPGPU Engine, Register Region
> Restrictions, page 844:
> 
>   "When source or destination datatype is 64b or operation is integer DWord
>    multiply, indirect addressing must not be used."
> 
> v2:
> - Fix it for Broxton too.
> 
> v3:
> - Simplify code by using subscript() and not creating a new num_components
> variable (Kenneth).
> 
> Signed-off-by: Samuel Iglesias Gonsálvez <siglesias at igalia.com>
> Cc: "12.0" <mesa-stable at lists.freedesktop.org>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=95462
> ---
>  src/mesa/drivers/dri/i965/brw_fs_nir.cpp | 20 +++++++++++++++++++-
>  1 file changed, 19 insertions(+), 1 deletion(-)
> 
> diff --git a/src/mesa/drivers/dri/i965/brw_fs_nir.cpp b/src/mesa/drivers/dri/i965/brw_fs_nir.cpp
> index d72b37b..20db075 100644
> --- a/src/mesa/drivers/dri/i965/brw_fs_nir.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_fs_nir.cpp
> @@ -3611,10 +3611,28 @@ fs_visitor::nir_emit_intrinsic(const fs_builder &bld, nir_intrinsic_instr *instr
>           unsigned read_size = instr->const_index[1] -
>              (instr->num_components - 1) * type_sz(dest.type);
>  
> +         fs_reg indirect_chv_high_32bit;
> +         brw_reg_type type = dest.type;
> +         bool is_chv_bxt_64bit =
> +            (devinfo->is_cherryview || devinfo->is_broxton) &&
> +            type_sz(dest.type) == 8;
> +         if (is_chv_bxt_64bit) {
> +            type = BRW_REGISTER_TYPE_UD;
> +            indirect_chv_high_32bit = vgrf(glsl_type::uint_type);
> +            /* Calculate indirect address to read high 32 bits */
> +            bld.ADD(indirect_chv_high_32bit, indirect, brw_imm_ud(4));
> +         }
> +
>           for (unsigned j = 0; j < instr->num_components; j++) {
>              bld.emit(SHADER_OPCODE_MOV_INDIRECT,
> -                     offset(dest, bld, j), offset(src, bld, j),
> +                     subscript(offset(dest, bld, j), type, 0), offset(src, bld, j),
>                       indirect, brw_imm_ud(read_size));
> +
> +            if (is_chv_bxt_64bit) {
> +               bld.emit(SHADER_OPCODE_MOV_INDIRECT,
> +                        subscript(offset(dest, bld, j), type, 1), offset(src, bld, j),
> +                        indirect_chv_high_32bit, brw_imm_ud(read_size));
> +            }
>           }

Although this code works fine, after talking with Kenneth in IRC, I am
going to refactor it to put it clearer:

if (!is_chv_bxt_64bit) {
   bld.emit(SHADER_OPCODE_MOV_INDIRECT,
            offset(dest, bld, j), offset(src, bld, j),
            indirect, brw_imm_ud(read_size));
} else {
  bld.emit(SHADER_OPCODE_MOV_INDIRECT,
          subscript(offset(dest, bld, j), BRW_REGISTER_TYPE_UD, 0),
          offset(src, bld, j),
          indirect, brw_imm_ud(read_size));

  bld.emit(SHADER_OPCODE_MOV_INDIRECT,
           subscript(offset(dest, bld, j), BRW_REGISTER_TYPE_UD, 1),
           offset(src, bld, j),
           indirect_chv_high_32bit, brw_imm_ud(read_size));
}

And remove 'type' variable.

What do you think Kenneth?

Sam

>        }
>        break;
> 

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: OpenPGP digital signature
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20160617/3aea8224/attachment.sig>