[Mesa-dev] [PATCH 7/7] i965/fs: Fix readInvocationARB and readFirstInvocationARB

[Matt Turner]

On Mon, Aug 28, 2017 at 7:51 AM, Jason Ekstrand <jason at jlekstrand.net> wrote:
> The readInvocationARB built-in maps fairly nicely to our BROADCAST
> opcode.  However, the current implementation isn't quite right.  This
> commit fixes three different issues:
>
>  1) It was blindly taking component 0 of the index value even if that
>     channel is disabled.  We need emit_uniformize() to fix this.

Do we have a test that this fixes?

>  2) It didn't handle invalid index values particularly gracefully.
>     These can actually cause assertion failures in the compiler because
>     we may try to access out-of-bounds on a register.  This commit
>     solves this by using invocation & (dispatch_width - 1).

For immediates, the fix seems fine and I assume that's the only thing
that can cause assertion failures.

For registers the behavior seems unspecified. I don't think we should
emit additional instructions for an unspecified case.

>  3) Neither readInvocationARB nor readFirstInvocationARB properly
>     handled 64-bit types because they manually stomped the destination
>     type to D regardless of bit-size.

"regardless of bit-size" is a bit misleading when all possible types
are 32-bit. That criticism isn't fair when the GL extension doesn't
require 64-bit types (and I don't know why we would patch the code to
support them before something actually exercises it).

I was hoping one of these changes somehow fixed FDO bug 101984
(because I'm out of guesses!), but no such luck. :(