[mips][ias] fix li macro when values are negated with ~

[sdardis]

Summary

The integrated assembler evaluates the expressions such as ~0x80000000 to
0xffffffff7fffffff early in the parsing process. This patch adds compatibility
with gas so that li loads the expected value (0x7fffffff) in those cases. This
only occurs iff all the upper 32bits are set and maintains existing checks by not
truncating the result down to 32 bits if any of the the upper bits are not set.

[dsanders]

After discussing it with you off-list and experimenting with our gcc toolchain, I think you're on the right track. We need to accept the immediate if it is a simm33 and render it as a uimm32. This isn't quite the same as the 'relaxed' predicates so we should rename things as appropriate. This is slightly odd since li technically produces a signed value on the target but this doesn't affect the target behaviour.

lib/Target/Mips/AsmParser/MipsAsmParser.cpp

1025–1035

This seems to be nearly equivalent to addConstantUImmOperands<32>(). The difference is this only truncates if the value is within 32-bit whereas addConstantUImmOperands<32> always truncates. I'd expect out-of-range numbers to have been rejected by the predicate. I think we should use addConstantUImmOperands<32> instead.

1089–1090

This predicate is wrong and I think it's a key reason (but not the only reason) you needed to add a new predicate and renderer. It currently evaluates to false for expressions like MCUnaryExpr('~', MCConstantExpr(0x7fffffff)) and can only be true for MCConstantExpr by itself. It should be calling evaluateAsAbsolute() and returning true if that's successful. getConstantImm() will need updating to match.

lib/Target/Mips/MipsInstrInfo.td

454

This should probably take the immediate size for consistency (like SImmAsmOperandClass)

458

I think this should be isSImm<33>

489

This isn't needed, we can inherit it from UImmAnyAsmOperandClass

511–514

I've just noticed that one of these should probably be have UImm16AsmOperandClass as the superclass instead of UImm16RelaxedAsmOperandClass.

[sdardis]

Address reviewer comments.

[sdardis]

Ping.

[sdardis]

Can you take a look at this Zoran? Thanks.

[sdardis]

Ping.

[zoran.jovanovic]

LGTM.

lib/Target/Mips/MipsInstrInfo.td

2384

Nit: Is this comment still valid?

[sdardis]

lib/Target/Mips/MipsInstrInfo.td

2384

No, it's not. I'll change it when I commit to "We use uimm32_coerced to accept a 33 bit signed number that is rendered into a 32 bit number." or similar.

[Eugene.Zelenko]

Looks like patch was not committed.

[sdardis]

Thanks, committed as r283353.

lib/Target/Mips/AsmParser/MipsAsmParser.cpp

void addHWRegsAsmRegOperands(MCInst &Inst, unsigned N) const {
     assert(N == 1 && "Invalid number of operands!");
     Inst.addOperand(MCOperand::createReg(getHWRegsReg()));
   }
 
   template <unsigned Bits, int Offset = 0, int AdjustOffset = 0>
   void addConstantUImmOperands(MCInst &Inst, unsigned N) const {
     assert(N == 1 && "Invalid number of operands!");
     uint64_t Imm = getConstantImm() - Offset;
-    Imm &= (1 << Bits) - 1;
+    Imm &= (1ULL << Bits) - 1;
     Imm += Offset;
     Imm += AdjustOffset;
     Inst.addOperand(MCOperand::createImm(Imm));
   }
 
   template <unsigned Bits>
   void addSImmOperands(MCInst &Inst, unsigned N) const {
     if (isImm() && !isConstantImm()) {
void addConstantSImmOperands(MCInst &Inst, unsigned N) const {
     assert(N == 1 && "Invalid number of operands!");
     int64_t Imm = getConstantImm() - Offset;
     Imm = SignExtend64<Bits>(Imm);
     Imm += Offset;
     Imm += AdjustOffset;
     Inst.addOperand(MCOperand::createImm(Imm));
   }
 
   void addImmOperands(MCInst &Inst, unsigned N) const {
     assert(N == 1 && "Invalid number of operands!");
     const MCExpr *Expr = getImm();
     addExpr(Inst, Expr);
   }
 
   void addMemOperands(MCInst &Inst, unsigned N) const {
     assert(N == 2 && "Invalid number of operands!");
 
     Inst.addOperand(MCOperand::createReg(AsmParser.getABI().ArePtrs64bit()
                                              ? getMemBase()->getGPR64Reg()
                                              : getMemBase()->getGPR32Reg()));
 
     const MCExpr *Expr = getMemOff();
     addExpr(Inst, Expr);
   }
 
   void addMicroMipsMemOperands(MCInst &Inst, unsigned N) const {
     assert(N == 2 && "Invalid number of operands!");
public:
   bool isReg() const override {
     // As a special case until we sort out the definition of div/divu, accept
     // $0/$zero here so that MCK_ZERO works correctly.
     return isGPRAsmReg() && RegIdx.Index == 0;
   }
   bool isRegIdx() const { return Kind == k_RegisterIndex; }
   bool isImm() const override { return Kind == k_Immediate; }
   bool isConstantImm() const {
-    return isImm() && isa<MCConstantExpr>(getImm());
+    int64_t Res;
+    return isImm() && getImm()->evaluateAsAbsolute(Res);
   }
   bool isConstantImmz() const {
     return isConstantImm() && getConstantImm() == 0;
   }
   template <unsigned Bits, int Offset = 0> bool isConstantUImm() const {
     return isConstantImm() && isUInt<Bits>(getConstantImm() - Offset);
   }
   template <unsigned Bits> bool isSImm() const {
public:
 
   const MCExpr *getImm() const {
     assert((Kind == k_Immediate) && "Invalid access!");
     return Imm.Val;
   }
 
   int64_t getConstantImm() const {
     const MCExpr *Val = getImm();
-    return static_cast<const MCConstantExpr *>(Val)->getValue();
+    int64_t Value = 0;
+    (void)Val->evaluateAsAbsolute(Value);
+    return Value;
   }
 
   MipsOperand *getMemBase() const {
     assert((Kind == k_Memory) && "Invalid access!");
     return Mem.Base;
   }
 
   const MCExpr *getMemOff() const {
return Error(RefineErrorLoc(IDLoc, Operands, ErrorInfo),
                  "expected 20-bit unsigned immediate");
   case Match_UImm26_0:
     return Error(RefineErrorLoc(IDLoc, Operands, ErrorInfo),
                  "expected 26-bit unsigned immediate");
   case Match_SImm32:
   case Match_SImm32_Relaxed:
     return Error(RefineErrorLoc(IDLoc, Operands, ErrorInfo),
                  "expected 32-bit signed immediate");
+  case Match_UImm32_Coerced:
+    return Error(RefineErrorLoc(IDLoc, Operands, ErrorInfo),
+                 "expected 32-bit immediate");
   case Match_MemSImm9:
     return Error(RefineErrorLoc(IDLoc, Operands, ErrorInfo),
                  "expected memory with 9-bit signed offset");
   case Match_MemSImm10:
     return Error(RefineErrorLoc(IDLoc, Operands, ErrorInfo),
                  "expected memory with 10-bit signed offset");
   case Match_MemSImm10Lsl1:
     return Error(RefineErrorLoc(IDLoc, Operands, ErrorInfo),

lib/Target/Mips/MipsInstrInfo.td

class UImmAsmOperandClass<int Bits, list<AsmOperandClass> Supers = []>
     : AsmOperandClass {
   let Name = "UImm" # Bits;
   let RenderMethod = "addUImmOperands<" # Bits # ">";
   let PredicateMethod = "isUImm<" # Bits # ">";
   let SuperClasses = Supers;
   let DiagnosticType = "UImm" # Bits;
 }
 
+// Generic case - only to support certain assembly pseudo instructions.
+class UImmAnyAsmOperandClass<int Bits, list<AsmOperandClass> Supers = []>
+    : AsmOperandClass {
+  let Name = "ImmAny";
+  let RenderMethod = "addConstantUImmOperands<32>";
+  let PredicateMethod = "isSImm<" # Bits # ">";
+  let SuperClasses = Supers;
+  let DiagnosticType = "ImmAny";
+}
+
 // AsmOperandClasses require a strict ordering which is difficult to manage
 // as a hierarchy. Instead, we use a linear ordering and impose an order that
 // is in some places arbitrary.
 //
 // Here the rules that are in use:
 // * Wider immediates are a superset of narrower immediates:
 //     uimm4 < uimm5 < uimm6
 // * For the same bit-width, unsigned immediates are a superset of signed
 //   immediates::
 //     simm4 < uimm4 < simm5 < uimm5
 // * For the same upper-bound, signed immediates are a superset of unsigned
 //   immediates:
 //     uimm3 < simm4 < uimm4 < simm4
 // * Modified immediates are a superset of ordinary immediates:
 //     uimm5 < uimm5_plus1 (1..32) < uimm5_plus32 (32..63) < uimm6
 //   The term 'superset' starts to break down here since the uimm5_plus* classes
 //   are not true supersets of uimm5 (but they are still subsets of uimm6).
 // * 'Relaxed' immediates are supersets of the corresponding unsigned immediate.
 //     uimm16 < uimm16_relaxed
 // * The codeGen pattern type is arbitrarily ordered.
 //     uimm5 < uimm5_64, and uimm5 < vsplat_uimm5
 //   This is entirely arbitrary. We need an ordering and what we pick is
 //   unimportant since only one is possible for a given mnemonic.
+
+def UImm32CoercedAsmOperandClass : UImmAnyAsmOperandClass<33, []> {
+  let Name = "UImm32_Coerced";
+  let DiagnosticType = "UImm32_Coerced";
+}
 def SImm32RelaxedAsmOperandClass
-    : SImmAsmOperandClass<32, []> {
+    : SImmAsmOperandClass<32, [UImm32CoercedAsmOperandClass]> {
   let Name = "SImm32_Relaxed";
   let PredicateMethod = "isAnyImm<32>";
   let DiagnosticType = "SImm32_Relaxed";
 }
 def SImm32AsmOperandClass
     : SImmAsmOperandClass<32, [SImm32RelaxedAsmOperandClass]>;
 def ConstantUImm26AsmOperandClass
     : ConstantUImmAsmOperandClass<26, [SImm32AsmOperandClass]>;
 def ConstantUImm20AsmOperandClass
     : ConstantUImmAsmOperandClass<20, [ConstantUImm26AsmOperandClass]>;
 def UImm16RelaxedAsmOperandClass
     : UImmAsmOperandClass<16, [ConstantUImm20AsmOperandClass]> {
   let Name = "UImm16_Relaxed";
   let PredicateMethod = "isAnyImm<16>";
   let DiagnosticType = "UImm16_Relaxed";
 }
+// FIXME: One of these should probably have UImm16AsmOperandClass as the
+//        superclass instead of UImm16RelaxedasmOPerandClass.
 def UImm16AsmOperandClass
     : UImmAsmOperandClass<16, [UImm16RelaxedAsmOperandClass]>;
 def SImm16RelaxedAsmOperandClass
     : SImmAsmOperandClass<16, [UImm16RelaxedAsmOperandClass]> {
   let Name = "SImm16_Relaxed";
   let PredicateMethod = "isAnyImm<16>";
   let DiagnosticType = "SImm16_Relaxed";
 }
 def SImm16AsmOperandClass
     : SImmAsmOperandClass<16, [SImm16RelaxedAsmOperandClass]>;
 def ConstantSImm10Lsl3AsmOperandClass : AsmOperandClass {
   let Name = "SImm10Lsl3";
def simm16_relaxed : Operand<i32> {
   let ParserMatchClass = !cast<AsmOperandClass>("SImm16RelaxedAsmOperandClass");
 }
 
 def simm16_64 : Operand<i64> {
   let DecoderMethod = "DecodeSImmWithOffsetAndScale<16>";
   let ParserMatchClass = !cast<AsmOperandClass>("SImm16AsmOperandClass");
 }
 
+// like simm32 but coerces simm32 to uimm32.
+def uimm32_coerced : Operand<i32> {
+  let ParserMatchClass = !cast<AsmOperandClass>("UImm32CoercedAsmOperandClass");
+}
 // Like simm32 but coerces uimm32 to simm32.
 def simm32_relaxed : Operand<i32> {
   let DecoderMethod = "DecodeSImmWithOffsetAndScale<32>";
   let ParserMatchClass = !cast<AsmOperandClass>("SImm32RelaxedAsmOperandClass");
 }
 
 // This is almost the same as a uimm7 but 0x7f is interpreted as -1.
 def li16_imm : Operand<i32> {

 }
 def : MipsInstAlias<"sdbbp", (SDBBP 0)>, ISA_MIPS32_NOT_32R6_64R6;
 def : MipsInstAlias<"sync",
                     (SYNC 0), 1>, ISA_MIPS2;
 //===----------------------------------------------------------------------===//
 // Assembler Pseudo Instructions
 //===----------------------------------------------------------------------===//
 
 // We use i32imm on li/la to defer range checking to the assembler.
 class LoadImmediate32<string instr_asm, Operand Od, RegisterOperand RO> :
   MipsAsmPseudoInst<(outs RO:$rt), (ins Od:$imm32),
                      !strconcat(instr_asm, "\t$rt, $imm32")> ;
-def LoadImm32 : LoadImmediate32<"li", i32imm, GPR32Opnd>;
+def LoadImm32 : LoadImmediate32<"li", uimm32_coerced, GPR32Opnd>;
 
 class LoadAddressFromReg32<string instr_asm, Operand MemOpnd,
                            RegisterOperand RO> :
   MipsAsmPseudoInst<(outs RO:$rt), (ins MemOpnd:$addr),
                      !strconcat(instr_asm, "\t$rt, $addr")> ;
 def LoadAddrReg32 : LoadAddressFromReg32<"la", mem, GPR32Opnd>;
 
 class LoadAddressFromImm32<string instr_asm, Operand Od, RegisterOperand RO> :

test/MC/Mips/macro-li-bad.s

 # RUN: not llvm-mc %s -arch=mips -mcpu=mips32r2 2>%t1
 # RUN: FileCheck %s < %t1 --check-prefix=32-BIT
 # RUN: not llvm-mc %s -arch=mips64 -mcpu=mips64 -target-abi n32 2>&1 | \
 # RUN:   FileCheck %s --check-prefix=64-BIT
 # RUN: not llvm-mc %s -arch=mips64 -mcpu=mips64 -target-abi n64 2>&1 | \
 # RUN:   FileCheck %s --check-prefix=64-BIT
 
   .text
   li $5, 0x100000000
-  # 32-BIT: :[[@LINE-1]]:3: error: instruction requires a 32-bit immediate
-  # 64-BIT: :[[@LINE-2]]:3: error: instruction requires a 32-bit immediate
+  # 32-BIT: :[[@LINE-1]]:10: error: expected 32-bit immediate
+  # 64-BIT: :[[@LINE-2]]:10: error: expected 32-bit immediate

test/MC/Mips/macro-li.s

 li $5, 0xffff4000 # CHECK: lui $5, 65535        # encoding: [0x3c,0x05,0xff,0xff]
                   # CHECK: ori $5, $5, 16384    # encoding: [0x34,0xa5,0x40,0x00]
 li $5, 0xfffe8000 # CHECK: lui $5, 65534        # encoding: [0x3c,0x05,0xff,0xfe]
                   # CHECK: ori $5, $5, 32768    # encoding: [0x34,0xa5,0x80,0x00]
 li $5, 0xc0008000 # CHECK: lui $5, 49152        # encoding: [0x3c,0x05,0xc0,0x00]
                   # CHECK: ori $5, $5, 32768    # encoding: [0x34,0xa5,0x80,0x00]
 li $5, 0x80008000 # CHECK: lui $5, 32768        # encoding: [0x3c,0x05,0x80,0x00]
                   # CHECK: ori $5, $5, 32768    # encoding: [0x34,0xa5,0x80,0x00]
+li $4, ~0xffffffff # CHECK; addiu $4, $zero, 0  # encoding: [0x24,0x04,0x00,0x00]
+li $4, ~0x80000001 # CHECK: lui $4, 32767       # encoding: [0x3c,0x04,0x7f,0xff]
+                   # CHECK: ori $4, $4, 65534   # encoding: [0x34,0x84,0xff,0xfe]
+li $4, ~0x80000000 # CHECK: lui $4, 32767       # encoding: [0x3c,0x04,0x7f,0xff]
+                   # CHECK: ori $4, $4, 65535   # encoding: [0x34,0x84,0xff,0xff]
+li $4, ~0x7fffffff # CHECK: lui $4, 32768       # encoding: [0x3c,0x04,0x80,0x00]
+li $4, ~0x00000001 # CHECK: addiu $4, $zero, -2 # encoding: [0x24,0x04,0xff,0xfe]
+li $4, ~0x00000000 # CHECK: addiu $4, $zero, -1 # encoding: [0x24,0x04,0xff,0xff]