Looks OK to me. I didn't think there was going to be much required to make this work -- is nice that it turned out to be nothing.
Reviewed-by: Chris Forbes <chr...@ijw.co.nz> - Chris On Fri, Jul 3, 2015 at 6:41 AM, Neil Roberts <n...@linux.intel.com> wrote: > 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."); > - > 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@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev