On Fri, Oct 27, 2017 at 12:09 AM, Iago Toral <ito...@igalia.com> wrote:
>
> On Wed, 2017-10-25 at 16:25 -0700, Jason Ekstrand wrote:
> > ---
> > src/intel/compiler/brw_fs.cpp | 10 ++++++++--
> > 1 file changed, 8 insertions(+), 2 deletions(-)
> >
> > diff --git a/src/intel/compiler/brw_fs.cpp
> > b/src/intel/compiler/brw_fs.cpp
> > index 1c4351b..52079d3 100644
> > --- a/src/intel/compiler/brw_fs.cpp
> > +++ b/src/intel/compiler/brw_fs.cpp
> > @@ -2416,8 +2416,14 @@ fs_visitor::opt_algebraic()
> > progress = true;
> > } else if (inst->src[1].file == IMM) {
> > inst->opcode = BRW_OPCODE_MOV;
> > - inst->src[0] = component(inst->src[0],
> > - inst->src[1].ud);
> > + /* It's possible that the selected component will be too
> > large and
> > + * overflow the register. If this happens and we some
> > how manage
> > + * to constant fold it in and get here, it would cause
> > an assert
> > + * in component() below. Instead, just let it wrap
> > around if it
> > + * goes over exec_size.
> > + */
>
> component() is really a horiz_offset() call which is in turn a
> byte_offset() call, which doesn't assert on anything other than invalid
> register files. I guess you mean that the byte offset computed by the
> component() call below can later lead to hitting assertions as we
> attempt to read outside the allocated space for the vgrf, right?
>
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.
> My question is whether this is supposed to happen at all, it seems like
> we should not be emitting broadcast operations like this at all since
> they are invalid and here we are instead papering over that bug.
>
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.
I've updated the comment to:
/* It's possible that the selected component will be too large
and
* overflow the register. This can happen if someone does a
* readInvocation() from GLSL or SPIR-V and provides an OOB
* invocationIndex. If this happens and we some how manage
* to constant fold it in and get here, then component() may
cause
* us to start reading outside of the VGRF which will lead to an
* assert later. Instead, just let it wrap around if it goes
over
* exec_size.
*/
--Jason
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
