On May 2, 2016 12:31 AM, "Iago Toral" <<a href="mailto:itoral@igalia.com">itoral@igalia.com</a>> wrote: > > On Sun, 2016-05-01 at 20:04 -0700, Jordan Justen wrote: > > On 2016-04-29 04:29:56, Samuel Iglesias Gonsálvez wrote: > > > In that case, the writes need two times the size of a 32-bit value. > > > We need to adjust the exec_size, so it is not breaking any hardware > > > rule. > > > > > > Signed-off-by: Samuel Iglesias Gonsálvez <<a href="mailto:siglesias@igalia.com">siglesias@igalia.com</a>> > > > --- > > > src/mesa/drivers/dri/i965/brw_fs.cpp | 8 +++++++- > > > 1 file changed, 7 insertions(+), 1 deletion(-) > > > > > > diff --git a/src/mesa/drivers/dri/i965/brw_fs.cpp b/src/mesa/drivers/dri/i965/brw_fs.cpp > > > index 059edc6..85d430c 100644 > > > --- a/src/mesa/drivers/dri/i965/brw_fs.cpp > > > +++ b/src/mesa/drivers/dri/i965/brw_fs.cpp > > > @@ -4655,7 +4655,13 @@ get_lowered_simd_width(const struct brw_device_info *devinfo, > > > > > > case SHADER_OPCODE_MOV_INDIRECT: > > > /* Prior to Broadwell, we only have 8 address subregisters */ > > > - return devinfo->gen < 8 ? 8 : MIN2(inst->exec_size, 16); > > > + if (devinfo->gen < 8) > > > + return 8; > > > + > > > + if (inst->exec_size < 16) > > > + return inst->exec_size; > > > + else > > > + return MIN2(inst->exec_size / (type_sz(inst->dst.type) / 4), 16); > > > > Can't type_sz return < 4, which could cause a divide by 0? If that > > can't happen along this code path, would an assert be a good idea? > > Looking at the places where the driver inserts MOV INDIRECT opcodes it > seems that it always uses region sizes that are multiples of 4 and dst > types are usually UD (except in load uniform where I don't think we can > see any types that are not at least 4 bytes, since we implement bool as > UD too), so it seems to me that we shouldn't be seeing types with sizes > < 4 here. At stone point in the future, it may be used for half-float types. I'm not to worried about that case at the moment though. > That said, I am adding Jason in the CC just in case I am missing > something. > > In any case, adding the assert for extra safety sounds good to me. > > > -Jordan > > > > > > > > default: > > > return inst->exec_size; > > > -- > > > 2.5.0 > > > > > > _______________________________________________ > > > 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">https://lists.freedesktop.org/mailman/listinfo/mesa-dev</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">https://lists.freedesktop.org/mailman/listinfo/mesa-dev</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">https://lists.freedesktop.org/mailman/listinfo/mesa-dev</a>