[Mesa-dev] [PATCH 1/3] nv50/ir: clean up src2 in ConstantFolding

[Ilia Mirkin]

On Fri, Dec 14, 2018 at 6:12 PM Karol Herbst <kherbst at redhat.com> wrote:
>
> no changes in shader-db
>
> Signed-off-by: Karol Herbst <kherbst at redhat.com>
> ---
>  src/gallium/drivers/nouveau/codegen/nv50_ir_peephole.cpp | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/src/gallium/drivers/nouveau/codegen/nv50_ir_peephole.cpp b/src/gallium/drivers/nouveau/codegen/nv50_ir_peephole.cpp
> index 5d3b4aba9cc..a8bd4f868bf 100644
> --- a/src/gallium/drivers/nouveau/codegen/nv50_ir_peephole.cpp
> +++ b/src/gallium/drivers/nouveau/codegen/nv50_ir_peephole.cpp
> @@ -763,6 +763,7 @@ ConstantFolding::expr(Instruction *i,
>        i->op = i->saturate ? OP_SAT : OP_MOV;
>        if (i->saturate)
>           unary(i, *i->getSrc(0)->asImm());
> +      i->setSrc(2, NULL);

Please preface this with a

if (i->srcExists(2)) ...

Otherwise it'll grow the array everywhere for nothing. This is just
for SLCT right? I don't think any other 3-operand ops would fall
through to here... this also seems a bit dangerous if an instruction
coming here might be predicated (which they usually wouldn't be until
later, but I think there are some flows where this could happen).

So I'd rather be explicit about which ops this is done for.
Unfortunately we don't have a map that tells us how many "regular"
operands an instruction should have... Looking over the list, I think
it's just OP_SLCT.

  -ilia

>        break;
>     }
>     i->subOp = 0;
> --
> 2.19.2
>