<div dir="ltr"><div>Series is </div>Reviewed-by: Jason Ekstrand <<a href="mailto:jason@jlekstrand.net" target="_blank">jason@jlekstrand.net</a>> </div><div class="gmail_extra"> <div class="gmail_quote">On Fri, May 4, 2018 at 10:45 AM, Matt Turner <<a href="mailto:mattst88@gmail.com" target="_blank">mattst88@gmail.com</a>> wrote: <blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="HOEnZb"><div class="h5">On Fri, May 4, 2018 at 4:10 AM, Iago Toral Quiroga <<a href="mailto:itoral@igalia.com">itoral@igalia.com</a>> wrote: > From: Jose Maria Casanova Crespo <<a href="mailto:jmcasanova@igalia.com">jmcasanova@igalia.com</a>> > > 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: <a href="https://bugs.freedesktop.org/show_bug.cgi?id=106393" rel="noreferrer" target="_blank">https://bugs.freedesktop.org/show_bug.cgi?id=106393</a> > --- > 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." > + */ </div></div>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 <<a href="mailto:mattst88@gmail.com">mattst88@gmail.com</a>> _______________________________________________ mesa-dev mailing list <a href="mailto:mesa-dev@lists.freedesktop.org">mesa-dev@lists.freedesktop.org</a> <a href="https://lists.freedesktop.org/mailman/listinfo/mesa-dev" rel="noreferrer" target="_blank">https://lists.freedesktop.org/mailman/listinfo/mesa-dev</a> </blockquote></div> </div>