On Sat, Apr 21, 2018 at 1:51 PM, Ilia Mirkin <imir...@alum.mit.edu> wrote:
> Some analysis suggests that all short immediates are sign-extended. The
> insnCanLoad logic already accounted for this, but we could still pick
> the wrong form when emitting actual instructions that support both short
> and long immediates (with the long form usually having additional
> restrictions that insnCanLoad should be aware of).
>
> This also reverses a bunch of commits that had previously "worked
> around" this issue in various emitters:
>
> 9c63224540ef: gm107/ir: make use of ADD32I for all immediates
> 83a4f28dc27b: gm107/ir: make use of LOP32I for all immediates
> b84c97587b4a: gm107/ir: make use of IMUL32I for all immediates
> d30768025a22: gk110/ir: make use of IMUL32I for all immediates
>
> as well as the original import for UMUL in the nvc0 emitter.
>
> Reported-by: Karol Herbst <kher...@redhat.com>
> Signed-off-by: Ilia Mirkin <imir...@alum.mit.edu>
> ---
> .../drivers/nouveau/codegen/nv50_ir_emit_gk110.cpp | 10 +++++++---
> .../drivers/nouveau/codegen/nv50_ir_emit_gm107.cpp | 21
> +++++++++------------
> .../drivers/nouveau/codegen/nv50_ir_emit_nvc0.cpp | 12 ++++++++----
> 3 files changed, 24 insertions(+), 19 deletions(-)
>
> diff --git a/src/gallium/drivers/nouveau/codegen/nv50_ir_emit_gk110.cpp
> b/src/gallium/drivers/nouveau/codegen/nv50_ir_emit_gk110.cpp
> index 370427d0d13..647d1a5d0ef 100644
> --- a/src/gallium/drivers/nouveau/codegen/nv50_ir_emit_gk110.cpp
> +++ b/src/gallium/drivers/nouveau/codegen/nv50_ir_emit_gk110.cpp
> @@ -207,7 +207,11 @@ bool CodeEmitterGK110::isLIMM(const ValueRef& ref,
> DataType ty, bool mod)
> {
> const ImmediateValue *imm = ref.get()->asImm();
>
> - return imm && (imm->reg.data.u32 & ((ty == TYPE_F32) ? 0xfff :
> 0xfff00000));
> + if (ty == TYPE_F32)
> + return imm && imm->reg.data.u32 & 0xfff;
> + else
> + return imm && (imm->reg.data.s32 > 0x7ffff ||
> + imm->reg.data.s32 < -0x80000);
> }
>
> void
> @@ -342,7 +346,7 @@ CodeEmitterGK110::setShortImmediate(const Instruction *i,
> const int s)
> code[1] |= ((u64 & 0x7fe0000000000000ULL) >> 53);
> code[1] |= ((u64 & 0x8000000000000000ULL) >> 36);
> } else {
> - assert((u32 & 0xfff00000) == 0 || (u32 & 0xfff00000) == 0xfff00000);
> + assert((u32 & 0xfff80000) == 0 || (u32 & 0xfff80000) == 0xfff80000);
> code[0] |= (u32 & 0x001ff) << 23;
> code[1] |= (u32 & 0x7fe00) >> 9;
> code[1] |= (u32 & 0x80000) << 8;
> @@ -633,7 +637,7 @@ CodeEmitterGK110::emitIMUL(const Instruction *i)
> assert(!i->src(0).mod.neg() && !i->src(1).mod.neg());
> assert(!i->src(0).mod.abs() && !i->src(1).mod.abs());
>
> - if (i->src(1).getFile() == FILE_IMMEDIATE) {
> + if (isLIMM(i->src(1), TYPE_S32)) {
> emitForm_L(i, 0x280, 2, Modifier(0));
>
> if (i->subOp == NV50_IR_SUBOP_MUL_HIGH)
> diff --git a/src/gallium/drivers/nouveau/codegen/nv50_ir_emit_gm107.cpp
> b/src/gallium/drivers/nouveau/codegen/nv50_ir_emit_gm107.cpp
> index fafece81ad0..e91b11fe214 100644
> --- a/src/gallium/drivers/nouveau/codegen/nv50_ir_emit_gm107.cpp
> +++ b/src/gallium/drivers/nouveau/codegen/nv50_ir_emit_gm107.cpp
> @@ -321,14 +321,10 @@ CodeEmitterGM107::longIMMD(const ValueRef &ref)
> {
> if (ref.getFile() == FILE_IMMEDIATE) {
> const ImmediateValue *imm = ref.get()->asImm();
> - if (isFloatType(insn->sType)) {
> - if ((imm->reg.data.u32 & 0x00000fff) != 0x00000000)
> - return true;
> - } else {
> - if ((imm->reg.data.u32 & 0xfff00000) != 0x00000000 &&
> - (imm->reg.data.u32 & 0xfff00000) != 0xfff00000)
> - return true;
> - }
> + if (isFloatType(insn->sType))
> + return imm->reg.data.u32 & 0xff;
That should of course be 0xfff. Fixed locally.
> + else
> + return imm->reg.data.s32 > 0x7ffff || imm->reg.data.s32 < -0x80000;
> }
> return false;
> }
> @@ -346,8 +342,9 @@ CodeEmitterGM107::emitIMMD(int pos, int len, const
> ValueRef &ref)
> } else if (insn->sType == TYPE_F64) {
> assert(!(imm->reg.data.u64 & 0x00000fffffffffffULL));
> val = imm->reg.data.u64 >> 44;
> + } else {
> + assert(!(val & 0xfff80000) || (val & 0xfff80000) == 0xfff80000);
> }
> - assert(!(val & 0xfff00000) || (val & 0xfff00000) == 0xfff00000);
> emitField( 56, 1, (val & 0x80000) >> 19);
> emitField(pos, len, (val & 0x7ffff));
> } else {
> @@ -1658,7 +1655,7 @@ CodeEmitterGM107::emitLOP()
> break;
> }
>
> - if (insn->src(1).getFile() != FILE_IMMEDIATE) {
> + if (!longIMMD(insn->src(1))) {
> switch (insn->src(1).getFile()) {
> case FILE_GPR:
> emitInsn(0x5c400000);
> @@ -1731,7 +1728,7 @@ CodeEmitterGM107::emitNOT()
> void
> CodeEmitterGM107::emitIADD()
> {
> - if (insn->src(1).getFile() != FILE_IMMEDIATE) {
> + if (!longIMMD(insn->src(1))) {
> switch (insn->src(1).getFile()) {
> case FILE_GPR:
> emitInsn(0x5c100000);
> @@ -1773,7 +1770,7 @@ CodeEmitterGM107::emitIADD()
> void
> CodeEmitterGM107::emitIMUL()
> {
> - if (insn->src(1).getFile() != FILE_IMMEDIATE) {
> + if (!longIMMD(insn->src(1))) {
> switch (insn->src(1).getFile()) {
> case FILE_GPR:
> emitInsn(0x5c380000);
> diff --git a/src/gallium/drivers/nouveau/codegen/nv50_ir_emit_nvc0.cpp
> b/src/gallium/drivers/nouveau/codegen/nv50_ir_emit_nvc0.cpp
> index be7ac182222..d85fdda56ff 100644
> --- a/src/gallium/drivers/nouveau/codegen/nv50_ir_emit_nvc0.cpp
> +++ b/src/gallium/drivers/nouveau/codegen/nv50_ir_emit_nvc0.cpp
> @@ -213,7 +213,11 @@ bool CodeEmitterNVC0::isLIMM(const ValueRef& ref,
> DataType ty)
> {
> const ImmediateValue *imm = ref.get()->asImm();
>
> - return imm && (imm->reg.data.u32 & ((ty == TYPE_F32) ? 0xfff :
> 0xfff00000));
> + if (ty == TYPE_F32)
> + return imm && imm->reg.data.u32 & 0xfff;
> + else
> + return imm && (imm->reg.data.s32 > 0x7ffff ||
> + imm->reg.data.s32 < -0x80000);
> }
>
> void
> @@ -352,7 +356,7 @@ CodeEmitterNVC0::setImmediate(const Instruction *i, const
> int s)
> } else
> if ((code[0] & 0xf) == 0x3 || (code[0] & 0xf) == 4) {
> // integer immediate
> - assert((u32 & 0xfff00000) == 0 || (u32 & 0xfff00000) == 0xfff00000);
> + assert((u32 & 0xfff80000) == 0 || (u32 & 0xfff80000) == 0xfff80000);
> assert(!(code[1] & 0xc000));
> u32 &= 0xfffff;
> code[0] |= (u32 & 0x3f) << 26;
> @@ -641,7 +645,7 @@ void
> CodeEmitterNVC0::emitUMUL(const Instruction *i)
> {
> if (i->encSize == 8) {
> - if (i->src(1).getFile() == FILE_IMMEDIATE) {
> + if (isLIMM(i->src(1), TYPE_U32)) {
> emitForm_A(i, HEX64(10000000, 00000002));
> } else {
> emitForm_A(i, HEX64(50000000, 00000003));
> @@ -2069,7 +2073,7 @@ CodeEmitterNVC0::emitMOV(const Instruction *i)
> assert(!(imm & 0x000fffff));
> code[0] = 0x00000318 | imm;
> } else {
> - assert(imm < 0x800 || ((int32_t)imm >= -0x800));
> + assert(imm < 0x800 && ((int32_t)imm >= -0x800));
> code[0] = 0x00000118 | (imm << 20);
> }
> } else {
> --
> 2.16.1
>
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
