[Mesa-dev] [PATCH v6 31/38] intel/compiler: implement SIMD16 restrictions for mixed-float instructions

[Francisco Jerez]

"Juan A. Suarez Romero" <jasuarez at igalia.com> writes:

> From: Iago Toral Quiroga <itoral at igalia.com>
>
> ---
>  src/intel/compiler/brw_fs.cpp | 65 +++++++++++++++++++++++++++++++++++
>  1 file changed, 65 insertions(+)
>
> diff --git a/src/intel/compiler/brw_fs.cpp b/src/intel/compiler/brw_fs.cpp
> index 2fc7793709b..3616a7afc31 100644
> --- a/src/intel/compiler/brw_fs.cpp
> +++ b/src/intel/compiler/brw_fs.cpp
> @@ -5607,6 +5607,42 @@ fs_visitor::lower_logical_sends()
>     return progress;
>  }
>  
> +static bool
> +is_mixed_float_with_fp32_dst(const fs_inst *inst)
> +{
> +   /* FIXME: This opcode sometimes uses :W type on the source
> +    * for some reason even if the operand is a half-float, we
> +    * should probably fix it to use the correct type.
> +    */
> +   if (inst->opcode == BRW_OPCODE_F16TO32)
> +      return true;
> +
> +   if (inst->dst.type != BRW_REGISTER_TYPE_F)
> +      return false;
> +
> +   for (int i = 0; i < inst->sources; i++) {
> +      if (inst->src[i].type == BRW_REGISTER_TYPE_HF)
> +         return true;
> +   }
> +
> +   return false;
> +}
> +
> +static bool
> +is_mixed_float_with_packed_fp16_dst(const fs_inst *inst)
> +{

F32TO16 is also sometimes used with a W type as destination IIRC (since
it's a Gen7 instruction that predates HF types).  Not sure if it's ever
packed, but it might be sensible to return true here if that's ever the
case.

Other than that looks okay to me.

> +   if (inst->dst.type != BRW_REGISTER_TYPE_HF ||
> +       inst->dst.stride != 1)
> +      return false;
> +
> +   for (int i = 0; i < inst->sources; i++) {
> +      if (inst->src[i].type == BRW_REGISTER_TYPE_F)
> +         return true;
> +   }
> +
> +   return false;
> +}
> +
>  /**
>   * Get the closest allowed SIMD width for instruction \p inst accounting for
>   * some common regioning and execution control restrictions that apply to FPU
> @@ -5769,6 +5805,35 @@ get_fpu_lowered_simd_width(const struct gen_device_info *devinfo,
>           max_width = MIN2(max_width, 4);
>     }
>  
> +   /* From the SKL PRM, Special Restrictions for Handling Mixed Mode
> +    * Float Operations:
> +    *
> +    *    "No SIMD16 in mixed mode when destination is f32. Instruction
> +    *     execution size must be no more than 8."
> +    *
> +    * FIXME: the simulator doesn't seem to complain if we don't do this and
> +    * empirical testing with existing CTS tests show that they pass just fine
> +    * without implementing this, however, since our interpretation of the PRM
> +    * is that conversion MOVs between HF and F are still mixed-float
> +    * instructions (and therefore subject to this restriction) we decided to
> +    * split them to be safe. Might be useful to do additional investigation to
> +    * lift the restriction if we can ensure that it is safe though, since these
> +    * conversions are common when half-float types are involved since many
> +    * instructions do not support HF types and conversions from/to F are
> +    * required.
> +    */
> +   if (is_mixed_float_with_fp32_dst(inst))
> +      max_width = MIN2(max_width, 8);
> +
> +   /* From the SKL PRM, Special Restrictions for Handling Mixed Mode
> +    * Float Operations:
> +    *
> +    *    "No SIMD16 in mixed mode when destination is packed f16 for both
> +    *     Align1 and Align16."
> +    */
> +   if (is_mixed_float_with_packed_fp16_dst(inst))
> +      max_width = MIN2(max_width, 8);
> +
>     /* Only power-of-two execution sizes are representable in the instruction
>      * control fields.
>      */
> -- 
> 2.20.1
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 227 bytes
Desc: not available
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20190327/cffb7573/attachment.sig>