[Mesa-dev] [PATCH] i965/fs: Don't disable SIMD16 when using the pixel interpolator

[Francisco Jerez]

Neil Roberts <neil at linux.intel.com> writes:

> There was a comment saying that in SIMD16 mode the pixel interpolator
> returns coords interleaved 8 channels at a time and that this requires
> extra work to support. However, this interleaved format is exactly
> what the PLN instruction requires so I don't think anything needs to
> be done to support it apart from removing the line to disable it and
> to ensure that the message lengths for the send message are correct.
>
> I am more convinced that this is correct because as it says in the
> comment this interleaved output is identical to what is given in the
> thread payload. The code generated to apply the plane equation to
> these coordinates is identical on SIMD16 and SIMD8 except that the
> dispatch width is larger which implies no special unmangling is
> needed.
>
> Perhaps the confusion stems from the fact that the description of the
> PLN instruction in the IVB PRM seems to imply that the src1 inputs are
> not interleaved so it wouldn't work. However, in the HSW and BDW PRMs,
> the pseudo-code is different and looks like it expects the interleaved
> format. Mesa doesn't seem to generate different code on IVB to
> uninterleave the payload registers and everything is working so I can
> only assume that the PRM is wrong.
>
> I tested the interpolateAt tests on HSW and did a full Piglit run on
> IVB on there were no regressions.
> ---
>
> I've CC'd Chris Forbes because according to git-annotate he wrote the
> original comment so he might know something I don't.
>
>  src/mesa/drivers/dri/i965/brw_fs_nir.cpp | 11 +++--------
>  1 file changed, 3 insertions(+), 8 deletions(-)
>
> diff --git a/src/mesa/drivers/dri/i965/brw_fs_nir.cpp b/src/mesa/drivers/dri/i965/brw_fs_nir.cpp
> index 59081ea..717e597 100644
> --- a/src/mesa/drivers/dri/i965/brw_fs_nir.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_fs_nir.cpp
> @@ -1461,12 +1461,6 @@ fs_visitor::nir_emit_intrinsic(const fs_builder &bld, nir_intrinsic_instr *instr
>     case nir_intrinsic_interp_var_at_centroid:
>     case nir_intrinsic_interp_var_at_sample:
>     case nir_intrinsic_interp_var_at_offset: {
> -      /* in SIMD16 mode, the pixel interpolator returns coords interleaved
> -       * 8 channels at a time, same as the barycentric coords presented in
> -       * the FS payload. this requires a bit of extra work to support.
> -       */
> -      no16("interpolate_at_* not yet supported in SIMD16 mode.");
> -

Heh, I happened to come across this comment yesterday while looking for
the remaining no16 calls and wondered why on earth it couldn't do the
same that the normal interpolation code does.  After this patch and a
series coming up that will remove all SIMD8 fallbacks from the texturing
code, the only case left still applicable to Gen7 hardware and later
will be "SIMD16 explicit accumulator operands unsupported".  Anyone?

>        fs_reg dst_xy = bld.vgrf(BRW_REGISTER_TYPE_F, 2);
>  
>        /* For most messages, we need one reg of ignored data; the hardware
> @@ -1531,7 +1525,7 @@ fs_visitor::nir_emit_intrinsic(const fs_builder &bld, nir_intrinsic_instr *instr
>                             bld.SEL(offset(src, i), itemp, fs_reg(7)));
>              }
>  
> -            mlen = 2;
> +            mlen = 2 * dispatch_width / 8;
>              inst = bld.emit(FS_OPCODE_INTERPOLATE_AT_PER_SLOT_OFFSET, dst_xy, src,
>                              fs_reg(0u));
>           }
> @@ -1543,7 +1537,8 @@ fs_visitor::nir_emit_intrinsic(const fs_builder &bld, nir_intrinsic_instr *instr
>        }
>  
>        inst->mlen = mlen;
> -      inst->regs_written = 2; /* 2 floats per slot returned */
> +      /* 2 floats per slot returned */
> +      inst->regs_written = 2 * dispatch_width / 8;
>        inst->pi_noperspective = instr->variables[0]->var->data.interpolation ==
>                                 INTERP_QUALIFIER_NOPERSPECTIVE;
>  
> -- 
> 1.9.3
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 212 bytes
Desc: not available
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20150703/f39374af/attachment.sig>