On Thu, Oct 13, 2016 at 9:53 AM, Samuel Pitoiset <samuel.pitoi...@gmail.com> wrote: > > > 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 <imir...@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...
That's a target-specific property. There could be some future hypothetical target that allows modifiers on shl. The check is cheap, so may as well :) If it were a more expensive check, I'd do it in a #if debug thing maybe. > > Looks good now. > > Reviewed-by: Samuel Pitoiset <samuel.pitoi...@gmail.com> Thanks! > > >> 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 _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev