[Mesa-stable] [Mesa-dev] [PATCH 4/5] intel/fs: Implement extended strides greater than 4 for IR source regions.

[Matt Turner]

On Thu, Feb 14, 2019 at 1:30 PM Jason Ekstrand <jason at jlekstrand.net> wrote:
>
> On Fri, Jan 18, 2019 at 6:09 PM Francisco Jerez <currojerez at 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 <jason at 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.