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

[Jason Ekstrand]

Series is

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



On Fri, May 4, 2018 at 10:45 AM, Matt Turner <mattst88 at gmail.com> wrote:

> 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>
> _______________________________________________
> 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/20180504/725c556a/attachment-0001.html>