<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Fri, Oct 27, 2017 at 12:09 AM, Iago Toral <span dir="ltr"><<a href="mailto:itoral@igalia.com" target="_blank">itoral@igalia.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div class="gmail-HOEnZb"><div class="gmail-h5"><br>
On Wed, 2017-10-25 at 16:25 -0700, Jason Ekstrand wrote:<br>
> ---<br>
>  src/intel/compiler/brw_fs.cpp | 10 ++++++++--<br>
>  1 file changed, 8 insertions(+), 2 deletions(-)<br>
><br>
> diff --git a/src/intel/compiler/brw_fs.<wbr>cpp<br>
> b/src/intel/compiler/brw_fs.<wbr>cpp<br>
> index 1c4351b..52079d3 100644<br>
> --- a/src/intel/compiler/brw_fs.<wbr>cpp<br>
> +++ b/src/intel/compiler/brw_fs.<wbr>cpp<br>
> @@ -2416,8 +2416,14 @@ fs_visitor::opt_algebraic()<br>
>              progress = true;<br>
>           } else if (inst->src[1].file == IMM) {<br>
>              inst->opcode = BRW_OPCODE_MOV;<br>
> -            inst->src[0] = component(inst->src[0],<br>
> -                             <wbr>        inst->src[1].ud);<br>
> +            /* It's possible that the selected component will be too<br>
> large and<br>
> +             * overflow the register.  If this happens and we some<br>
> how manage<br>
> +             * to constant fold it in and get here, it would cause<br>
> an assert<br>
> +             * in component() below.  Instead, just let it wrap<br>
> around if it<br>
> +             * goes over exec_size.<br>
> +             */<br>
<br>
</div></div>component() is really a horiz_offset() call which is in turn a<br>
byte_offset() call, which doesn't assert on anything other than invalid<br>
register files. I guess you mean that the byte offset computed by the<br>
component() call below can later lead to hitting assertions as we<br>
attempt to read outside the allocated space for the vgrf, right?<br></blockquote><div><br></div><div>Yes.  Nothing asserts here, we just end up reading way past the end of the VGRF and the validator which checks whether or not we keep all our reads in-bounds throws a fit.<br></div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
My question is whether this is supposed to happen at all, it seems like<br>
we should not be emitting broadcast operations like this at all since<br>
they are invalid and here we are instead papering over that bug.<br>
</blockquote></div></div><div class="gmail_extra"><br></div><div class="gmail_extra">This can happen if someone does a read_invocation from SPIR-V or GLSL that has an invocation index that is too large.  We're allowed to give them garbage values, but asserting isn't nice.</div><div class="gmail_extra"><br></div><div class="gmail_extra">I've updated the comment to:</div><div class="gmail_extra"><br></div><div class="gmail_extra">            /* It's possible that the selected component will be too large and<br>             * overflow the register.  This can happen if someone does a<br>             * readInvocation() from GLSL or SPIR-V and provides an OOB<br>             * invocationIndex.  If this happens and we some how manage<br>             * to constant fold it in and get here, then component() may cause<br>             * us to start reading outside of the VGRF which will lead to an<br>             * assert later.  Instead, just let it wrap around if it goes over<br>             * exec_size.<br>             */<br></div><div class="gmail_extra"><br></div><div class="gmail_extra">--Jason<br></div></div>