[Mesa-dev] [PATCH] nv50/ir: Check for valid insn instead of defs size

[Pierre Moreau]

On 06:10 PM - Mar 08 2016, Ilia Mirkin wrote:
> Patch is fine, description is wrong (or at least inaccurate).
> 
> The real issue is that function arguments have defs, but no defining
> instruction. As a result, there's nothing to do when allocating
> registers. This has nothing to do with $r0, but it does have something
> to do with the fact that nv50 compute makes use of function arguments
> for compute programs.

That's what I meant to write, but reading it again, it is confusing. I'll
rewrite it.

Pierre

> 
>   -ilia
> 
> On Wed, Feb 24, 2016 at 8:03 PM, Pierre Moreau <pierre.morrow at free.fr> wrote:
> > On Tesla cards, the first register $r0 contains the thread id; later
> > generations use a specialised register for it. In order to prevent the register
> > from being given to anyone, and thus lose the thread id information, an lvalue
> > is created to represent $r0 and is passed as an argument to the `main`
> > function.
> >
> > However, since the inputs and outputs of a function are stored as value
> > definitions, a definition is added onto the previously created lvalue without
> > it being associated to an instruction. Therefore, checking the number of
> > definitions of an lvalue do not ensure that it is associated to an instruction.
> >
> > Fixes a nullptr dereference in the register allocation pass, while running
> > compute kernels that do not use $r0.
> >
> > Signed-off-by: Pierre Moreau <pierre.morrow at free.fr>
> > ---
> >  src/gallium/drivers/nouveau/codegen/nv50_ir_ra.cpp | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/src/gallium/drivers/nouveau/codegen/nv50_ir_ra.cpp b/src/gallium/drivers/nouveau/codegen/nv50_ir_ra.cpp
> > index d877c25..500ab89 100644
> > --- a/src/gallium/drivers/nouveau/codegen/nv50_ir_ra.cpp
> > +++ b/src/gallium/drivers/nouveau/codegen/nv50_ir_ra.cpp
> > @@ -853,7 +853,7 @@ isShortRegOp(Instruction *insn)
> >  static bool
> >  isShortRegVal(LValue *lval)
> >  {
> > -   if (lval->defs.size() == 0)
> > +   if (lval->getInsn() == NULL)
> >        return false;
> >     for (Value::DefCIterator def = lval->defs.begin();
> >          def != lval->defs.end(); ++def)
> > @@ -1467,7 +1467,7 @@ GCRA::allocateRegisters(ArrayList& insns)
> >           nodes[i].init(regs, lval);
> >           RIG.insert(&nodes[i]);
> >
> > -         if (lval->inFile(FILE_GPR) && lval->defs.size() > 0 &&
> > +         if (lval->inFile(FILE_GPR) && lval->getInsn() != NULL &&
> >               prog->getTarget()->getChipset() < 0xc0) {
> >              Instruction *insn = lval->getInsn();
> >              if (insn->op == OP_MAD || insn->op == OP_SAD)
> > --
> > 2.7.1
> >
> > _______________________________________________
> > mesa-dev mailing list
> > mesa-dev at lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/mesa-dev
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20160309/4cee86a2/attachment.sig>