<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Mon, Aug 28, 2017 at 2:50 PM, Matt Turner <span dir="ltr"><<a href="mailto:mattst88@gmail.com" target="_blank">mattst88@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><span class="">On Mon, Aug 28, 2017 at 7:51 AM, Jason Ekstrand <<a href="mailto:jason@jlekstrand.net">jason@jlekstrand.net</a>> wrote:<br>
> The readInvocationARB built-in maps fairly nicely to our BROADCAST<br>
> opcode.  However, the current implementation isn't quite right.  This<br>
> commit fixes three different issues:<br>
><br>
>  1) It was blindly taking component 0 of the index value even if that<br>
>     channel is disabled.  We need emit_uniformize() to fix this.<br>
<br>
</span>Do we have a test that this fixes?<span class=""><br></span></blockquote><div><br></div><div>I don't believe so.<br></div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><span class="">
>  2) It didn't handle invalid index values particularly gracefully.<br>
>     These can actually cause assertion failures in the compiler because<br>
>     we may try to access out-of-bounds on a register.  This commit<br>
>     solves this by using invocation & (dispatch_width - 1).<br>
<br>
</span>For immediates, the fix seems fine and I assume that's the only thing<br>
that can cause assertion failures.<br></blockquote><div><br></div><div>Yes, that's correct.<br></div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
For registers the behavior seems unspecified. I don't think we should<br>
emit additional instructions for an unspecified case.<span class=""><br></span></blockquote><div><br></div><div>Unless that unspecified behavior is a GPU hang which I believe is possible if your register indirect goes into the weeds.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><span class="">
>  3) Neither readInvocationARB nor readFirstInvocationARB properly<br>
>     handled 64-bit types because they manually stomped the destination<br>
>     type to D regardless of bit-size.<br>
<br>
</span>"regardless of bit-size" is a bit misleading when all possible types<br>
are 32-bit. That criticism isn't fair when the GL extension doesn't<br>
require 64-bit types (and I don't know why we would patch the code to<br>
support them before something actually exercises it).<br></blockquote><div><br></div><div>Sure.  I could reword things and say that it makes it ready for 64-bits.  The only real "fix" there is to be more careful with our retyping.</div><div><br></div><div>--Jason<br></div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
I was hoping one of these changes somehow fixed FDO bug 101984<br>
(because I'm out of guesses!), but no such luck. :(<br>
</blockquote></div><br></div></div>