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

[Ilia Mirkin]

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.

  -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