[Mesa-dev] [PATCH v2] nvc0/ir: be more careful about preserving modifiers in SHLADD creation

[Samuel Pitoiset]

On 10/12/2016 08:42 PM, Ilia Mirkin wrote:
> src2 was being given the wrong modifier, and we were not properly
> managing the modifier on the SHL source either.
>
> Signed-off-by: Ilia Mirkin <imirkin at alum.mit.edu>
> ---
>  src/gallium/drivers/nouveau/codegen/nv50_ir_peephole.cpp | 12 +++++-------
>  1 file changed, 5 insertions(+), 7 deletions(-)
>
> diff --git a/src/gallium/drivers/nouveau/codegen/nv50_ir_peephole.cpp b/src/gallium/drivers/nouveau/codegen/nv50_ir_peephole.cpp
> index d88bb34..737bda3 100644
> --- a/src/gallium/drivers/nouveau/codegen/nv50_ir_peephole.cpp
> +++ b/src/gallium/drivers/nouveau/codegen/nv50_ir_peephole.cpp
> @@ -2163,7 +2163,6 @@ LateAlgebraicOpt::tryADDToSHLADD(Instruction *add)
>     Value *src1 = add->getSrc(1);
>     ImmediateValue imm;
>     Instruction *shl;
> -   Modifier mod[2];
>     Value *src;
>     int s;
>
> @@ -2182,20 +2181,19 @@ LateAlgebraicOpt::tryADDToSHLADD(Instruction *add)
>     src = add->getSrc(s);
>     shl = src->getUniqueInsn();
>
> -   if (shl->bb != add->bb || shl->usesFlags() || shl->subOp)
> +   if (shl->bb != add->bb || shl->usesFlags() || shl->subOp || shl->src(0).mod)

SHL can't have any modifiers, why do you check it here? I would say it's 
just for safety, but if the compiler adds modifiers to SHL something is 
really wrong...

Looks good now.

Reviewed-by: Samuel Pitoiset <samuel.pitoiset at gmail.com>

>        return false;
>
>     if (!shl->src(1).getImmediate(imm))
>        return false;
>
> -   mod[0] = add->src(0).mod;
> -   mod[1] = add->src(1).mod;
> -
>     add->op = OP_SHLADD;
>     add->setSrc(2, add->src(!s));
> -   add->src(2).mod = mod[s];
> -
> +   // SHL can't have any modifiers, but the ADD source may have had
> +   // one. Preserve it.
>     add->setSrc(0, shl->getSrc(0));
> +   if (s == 1)
> +      add->src(0).mod = add->src(1).mod;
>     add->setSrc(1, new_ImmediateValue(shl->bb->getProgram(), imm.reg.data.u32));
>     add->src(1).mod = Modifier(0);
>
>

-- 
-Samuel