[Mesa-dev] [PATCH] nvc0/ir: all short immediates are sign-extended, adjust LIMM test

Ilia Mirkin imirkin at alum.mit.edu
Sat Apr 21 18:34:30 UTC 2018


On Sat, Apr 21, 2018 at 1:51 PM, Ilia Mirkin <imirkin at 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 <kherbst at redhat.com>
> Signed-off-by: Ilia Mirkin <imirkin at 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
>


More information about the mesa-dev mailing list