[mips][ias] Make immediate zero synonymous with $zero when parsing registers.

[dsanders]

Summary

The constant zero is now usable wherever a GPR is expected and will be treated as $zero.

This fixes PR22389.

[tomatabacu]

I'm not sure if we should do this.
This patch would make us accept very strange instructions, such as "add 0, $4, $4", and it does not address the root of the issue, which is the IAS' lack of a BNE+Imm macro instruction.
GAS has such a macro, so it doesn't complain about Ed's use case (and also accepts any other immediates).

If Ed thinks it would be useful for him to have his particular use case fixed sooner rather than later, then LGTM with the inlined changes.
Otherwise, I think doing a proper implementation of the BNE+Imm macro would be best (which we should do at some point either way).

Also, adding some FIXME's to the source code which mention the BNE+Imm macro would be useful.

lib/Target/Mips/AsmParser/MipsAsmParser.cpp

898–900

Actually, I believe you can do this because you have access to the MipsAsmParser, so you can call AsmParser.isGP64bit().

test/MC/Mips/zero.s

2

Mention that this test is for PR22389.

6–7

Add the example from the Bugzilla ticket (bne $2, 0x0, foo).

[dsanders]

I'm not sure if we should do this.
This patch would make us accept very strange instructions, such as "add 0, $4, $4"

I hadn't thought of that one. I was thinking of things like 'addu $2, 0, $2' and instructions that don't have immediate forms.

I'm a little on the fence as to whether I agree it's strange or not. True, it's syntax that GAS doesn't accept but I like the way it suggests that the result is still zero after the instruction because constants don't change.

and it does not address the root of the issue, which is the IAS' lack of a BNE+Imm macro instruction.
GAS has such a macro, so it doesn't complain about Ed's use case (and also accepts any other immediates).

If Ed thinks it would be useful for him to have his particular use case fixed sooner rather than later, then LGTM with the inlined changes.
Otherwise, I think doing a proper implementation of the BNE+Imm macro would be best (which we should do at some point either way).

You've also touched on another good reason to not do this patch there. If it generates different opcodes compared to the immediate forms of instructions then it will be harder to verify our assembler by comparing object files.

[tomatabacu]

I'm a little on the fence as to whether I agree it's strange or not.

I'm just not keen on introducing such an exception to our handling of the MIPS assembly "type" system.

However, as you yourself have mentioned, there are other stronger reasons to not make this change.

lib/Target/Mips/AsmParser/MipsAsmParser.cpp

static std::unique_ptr<MipsOperand> CreateReg(unsigned Index, RegKind RegKind,
     Op->EndLoc = E;
     return Op;
   }
 
 public:
   /// Coerce the register to GPR32 and return the real register for the current
   /// target.
   unsigned getGPR32Reg() const {
+    // As a special case, immediate zero is also register zero.
+    if (isConstantImm() && getConstantImm() == 0)
+      return Mips::ZERO;
+
     assert(isRegIdx() && (RegIdx.Kind & RegKind_GPR) && "Invalid access!");
     AsmParser.warnIfAssemblerTemporary(RegIdx.Index, StartLoc);
     unsigned ClassID = Mips::GPR32RegClassID;
     return RegIdx.RegInfo->getRegClass(ClassID).getRegister(RegIdx.Index);
   }
 
   /// Coerce the register to GPR32 and return the real register for the current
   /// target.
   unsigned getGPRMM16Reg() const {
+    // As a special case, immediate zero is also register zero.
+    if (isConstantImm() && getConstantImm() == 0)
+      return Mips::ZERO;
+
     assert(isRegIdx() && (RegIdx.Kind & RegKind_GPR) && "Invalid access!");
     unsigned ClassID = Mips::GPR32RegClassID;
     return RegIdx.RegInfo->getRegClass(ClassID).getRegister(RegIdx.Index);
   }
 
   /// Coerce the register to GPR64 and return the real register for the current
   /// target.
   unsigned getGPR64Reg() const {
+    // As a special case, immediate zero is also register zero.
+    if (isConstantImm() && getConstantImm() == 0)
+      return Mips::ZERO_64;
+
     assert(isRegIdx() && (RegIdx.Kind & RegKind_GPR) && "Invalid access!");
     unsigned ClassID = Mips::GPR64RegClassID;
     return RegIdx.RegInfo->getRegClass(ClassID).getRegister(RegIdx.Index);
   }
 
 private:
   /// Coerce the register to AFGR64 and return the real register for the current
   /// target.
public:
   }
 
   bool isReg() const override {
     // As a special case until we sort out the definition of div/divu, pretend
     // that $0/$zero are k_PhysRegister so that MCK_ZERO works correctly.
     if (isGPRAsmReg() && RegIdx.Index == 0)
       return true;
 
+    // As another special case, immediate zero is also register zero.
+    if (isConstantImm() && getConstantImm() == 0)
+      return true;
+
     return Kind == k_PhysRegister;
   }
   bool isRegIdx() const { return Kind == k_RegisterIndex; }
   bool isImm() const override { return Kind == k_Immediate; }
   bool isConstantImm() const {
     return isImm() && dyn_cast<MCConstantExpr>(getImm());
   }
   bool isToken() const override {
public:
 
   unsigned getReg() const override {
     // As a special case until we sort out the definition of div/divu, pretend
     // that $0/$zero are k_PhysRegister so that MCK_ZERO works correctly.
     if (Kind == k_RegisterIndex && RegIdx.Index == 0 &&
         RegIdx.Kind & RegKind_GPR)
       return getGPR32Reg(); // FIXME: GPR64 too
 
+    // As another special case, immediate zero is also register zero.
+    // FIXME: We ought to handle the GPR64 case properly but MipsOperand
+    //        doesn't know the subtarget and the assembler doesn't need
+    //        that much precision anyway.
+    if (isConstantImm() && getConstantImm() == 0)
+      return Mips::ZERO;
+
     assert(Kind == k_PhysRegister && "Invalid access!");
     return PhysReg.Num;
   }
 
   const MCExpr *getImm() const {
     assert((Kind == k_Immediate) && "Invalid access!");
     return Imm.Val;
   }
CreateRegPair(unsigned RegNo, SMLoc S, SMLoc E, MipsAsmParser &Parser) {
     auto Op = make_unique<MipsOperand>(k_RegPair, Parser);
     Op->RegIdx.Index = RegNo;
     Op->StartLoc = S;
     Op->EndLoc = E;
     return Op;
   }
 
   bool isGPRAsmReg() const {
+    // As a special case, immediate zero is also register zero.
+    if (isConstantImm() && getConstantImm() == 0)
+      return true;
+
     return isRegIdx() && RegIdx.Kind & RegKind_GPR && RegIdx.Index <= 31;
   }
   bool isMM16AsmReg() const {
     if (!(isRegIdx() && RegIdx.Kind))
       return false;
     return ((RegIdx.Index >= 2 && RegIdx.Index <= 7)
             || RegIdx.Index == 16 || RegIdx.Index == 17);
   }

test/MC/Mips/zero.s

+// RUN: llvm-mc -arch=mips -show-encoding %s -o - | FileCheck %s
+
+  .text
+  bne $2, $0, foo     # CHECK: bnez $2, foo # encoding: [0x14,0x40,A,A]
+  bne $2, $zero, foo  # CHECK: bnez $2, foo # encoding: [0x14,0x40,A,A]
+  bne $2, 0, foo      # CHECK: bnez $2, foo # encoding: [0x14,0x40,A,A]
+foo: