[Mesa-dev] [PATCH 3/4] intel/compiler: Fix lower_conversions for 8-bit types.

[Matt Turner]

On Fri, May 4, 2018 at 4:10 AM, Iago Toral Quiroga <itoral at igalia.com> wrote:
> From: Jose Maria Casanova Crespo <jmcasanova at igalia.com>
>
> For 8-bit types the execution type is word. A byte raw MOV has 16-bit
> execution type and 8-bit destination and it shouldn't be considered
> a conversion case. So there is no need to change alignment and enter
> in lower_conversions for these instructions.
>
> Fixes a regresion in the piglit test "glsl-fs-shader-stencil-export"
> that is introduced with this patch from the Vulkan shaderInt16 series:
> 'i965/compiler: handle conversion to smaller type in the lowering
> pass for that'. The problem is caused because there is already a case
> in the driver that injects Byte instructions like this:
>
> mov(8)          g127<1>UB       g2<32,8,4>UB
>
> And the aforementioned pass was not accounting for the special
> handling of the execution size of Byte instructions. This patch
> fixes this.
>
> v2: (Jason Ekstrand)
>    - Simplify is_byte_raw_mov, include reference to PRM and not
>    consider B <-> UB conversions as raw movs.
>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=106393
> ---
>  src/intel/compiler/brw_fs_lower_conversions.cpp | 21 ++++++++++++++++++++-
>  1 file changed, 20 insertions(+), 1 deletion(-)
>
> diff --git a/src/intel/compiler/brw_fs_lower_conversions.cpp b/src/intel/compiler/brw_fs_lower_conversions.cpp
> index f95b39d3e8..f6c936cf21 100644
> --- a/src/intel/compiler/brw_fs_lower_conversions.cpp
> +++ b/src/intel/compiler/brw_fs_lower_conversions.cpp
> @@ -43,6 +43,24 @@ supports_type_conversion(const fs_inst *inst) {
>     }
>  }
>
> +/* From the SKL PRM Vol 2a, "Move":
> + *
> + *    "A mov with the same source and destination type, no source modifier,
> + *    and no saturation is a raw move. A packed byte destination region (B
> + *    or UB type with HorzStride == 1 and ExecSize > 1) can only be written
> + *    using raw move."
> + */

Indent the last three lines one more space to align vertically with
the first line.

> +static bool
> +is_byte_raw_mov (const fs_inst *inst)
> +{
> +   return type_sz(inst->dst.type) == 1 &&
> +      inst->opcode == BRW_OPCODE_MOV &&
> +      inst->src[0].type == inst->dst.type &&
> +      !inst->saturate &&
> +      !inst->src[0].negate &&
> +      !inst->src[0].abs;

And indent these to align vertically as well.

The first three patches are

Reviewed-by: Matt Turner <mattst88 at gmail.com>