On Thu, Feb 14, 2019 at 1:30 PM Jason Ekstrand <ja...@jlekstrand.net> wrote: > > On Fri, Jan 18, 2019 at 6:09 PM Francisco Jerez <curroje...@riseup.net> wrote: >> >> Strides up to 32B can be implemented for the source regions of most >> instructions by leveraging either the vertical or the horizontal >> stride of the hardware Align1 region. The main motivation for this is >> that currently the lower_integer_multiplication() pass will happily >> double the stride of one of the 32-bit sources, which can blow up if >> the stride of the original source was already the maximum value >> allowed by the hardware. > > > I thought this looked familiar so I did some digging... > > On Nov 2 of 2017, I wrote almost exactly this same patch which was committed > on Nov 7 as e8c9e65185de3e821e1 > On Nov 14, Matt reverted it in a31d0382084c8aa8 because it wasn't needed > anymore and he wasn't sure of its correctness. > > And here we are again.... > > I still believe it to be correct so it is > > Reviewed-by: Jason Ekstrand <ja...@jlekstrand.net> > > My one major request is that you include some of the history of this change > in the commit message. As far as the patch itself goes, it's identical to > mine except for the unneeded whitespace change and one additional assert > which I believe to be a good addition. > > I've also CC'd matt in case he wants to throw in his $.02
I don't think I have time to give this a thoughtful review, but as far as I can remember (with the commit messages to help) the patch I reverted was the wrong fix at the time. It left some tests failing, which I fixed in commit 6ac2d1690192, at which point the other commit was reverted since it was dead code. Perhaps you've found another case that needs it, but I think it was the right decision at the time. _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
