[mips] [IAS] Add support for LAReg with identical source and destination register operands.

[tomatabacu]

Summary

In this case, we're supposed to load the immediate in AT and then ADDu it with the source register and put it in the destination register.

[tomatabacu]

Rebased.

[dsanders]

lib/Target/Mips/AsmParser/MipsAsmParser.cpp

1757–1758

There's no need for UseAT. It's equivalent to ATReg == 0

1803

The 'UseAT ? ATReg : DstReg' is repeated a number of times. Please use a variable to represent the temporary register.

test/MC/Mips/mips-expansions.s

196

I think this should be:

addiu $8, $8, 20

[tomatabacu]

Addressed review comments.
Rebased (for Sean Bruno's convenience).

[tomatabacu]

lib/Target/Mips/AsmParser/MipsAsmParser.cpp

1757–1758

Fixed.

1803

Fixed.

test/MC/Mips/mips-expansions.s

196

I thought this was reported as a bug to the GAS developers and they said that they would switch to also use ORi.
Even if we decide to change it, it should be in a separate patch.

[dsanders]

LGTM with a nit

lib/Target/Mips/AsmParser/MipsAsmParser.cpp

1765

Nit: We don't need to use ATReg again so you can fold that ternary operator into the control flow above.

test/MC/Mips/mips-expansions.s

196

I'd forgotten about that, thanks for reminding me.

It's not exactly a bug. If I remember my conversation with Matthew correctly, there was some benefit to favoring addiu in the really old CPU's but that benefit no longer applies to anything in use today.

[tomatabacu]

Moved ATReg inside the if block, as it is not used again after that.
Shortened the name of IntermediateDstReg to TmpReg.
Rebased on top of the new D9523, D9348 and D9366.
Removed unnecessary dependencies on other patches.

[tomatabacu]

Rebased.

Daniel, is shortening IntermediateDstReg to TmpReg OK with you ?
You pointed that out in private, but since I ended up making the change, your position should be included in the public review.

[dsanders]

LGTM

Daniel, is shortening IntermediateDstReg to TmpReg OK with you ?
You pointed that out in private, but since I ended up making the change, your position should be included in the public review.

I didn't mind either way. When we spoke, I was making a general comment that descriptive names are generally good but short, non-descriptive names aren't always bad. The obvious example is 'i', and 'j' for loop counters. Personally, I use TmpReg for trivial scratch registers and use more descriptive names when there are several of them (otherwise it starts to get confusing).

lib/Target/Mips/AsmParser/MipsAsmParser.cpp

bool MipsAsmParser::loadImmediate(int64_t ImmValue, unsigned DstReg,
   }
 
   bool UseSrcReg = false;
   if (SrcReg != Mips::NoRegister)
     UseSrcReg = true;
 
   MCInst tmpInst;
 
+  unsigned ATReg = 0;
+  if (UseSrcReg && (DstReg == SrcReg)) {
+    // At this point we need AT to perform the expansions and we exit if it is
+    // not available.
+    ATReg = getATReg(IDLoc);
+    if (!ATReg)
+      return true;
+  }
+  unsigned IntermediateDstReg = (ATReg != 0) ? ATReg : DstReg;
+
   tmpInst.setLoc(IDLoc);
   // FIXME: gas has a special case for values that are 000...1111, which
   // becomes a li -1 and then a dsrl
   if (0 <= ImmValue && ImmValue <= 65535) {
     // For unsigned and positive signed 16-bit values (0 <= j <= 65535):
     // li d,j => ori d,$zero,j
     if (!UseSrcReg)
       SrcReg = isGP64bit() ? Mips::ZERO_64 : Mips::ZERO;
if (0 <= ImmValue && ImmValue <= 65535) {
     //           ori d,d,lo16(j)
     uint16_t Bits31To16 = (ImmValue >> 16) & 0xffff;
     uint16_t Bits15To0 = ImmValue & 0xffff;
 
     if (!Is32BitImm && !isInt<32>(ImmValue)) {
       // For DLI, expand to an ORi instead of a LUi to avoid sign-extending the
       // upper 32 bits.
       tmpInst.setOpcode(Mips::ORi);
-      tmpInst.addOperand(MCOperand::createReg(DstReg));
+      tmpInst.addOperand(MCOperand::createReg(IntermediateDstReg));
       tmpInst.addOperand(MCOperand::createReg(Mips::ZERO));
       tmpInst.addOperand(MCOperand::createImm(Bits31To16));
       tmpInst.setLoc(IDLoc);
       Instructions.push_back(tmpInst);
       // Move the value to the upper 16 bits by doing a 16-bit left shift.
-      createLShiftOri<16>(0, DstReg, IDLoc, Instructions);
+      createLShiftOri<16>(0, IntermediateDstReg, IDLoc, Instructions);
     } else {
       tmpInst.setOpcode(Mips::LUi);
-      tmpInst.addOperand(MCOperand::createReg(DstReg));
+      tmpInst.addOperand(MCOperand::createReg(IntermediateDstReg));
       tmpInst.addOperand(MCOperand::createImm(Bits31To16));
       Instructions.push_back(tmpInst);
     }
-    createLShiftOri<0>(Bits15To0, DstReg, IDLoc, Instructions);
+    createLShiftOri<0>(Bits15To0, IntermediateDstReg, IDLoc, Instructions);
 
     if (UseSrcReg)
-      createAddu(DstReg, DstReg, SrcReg, Instructions);
+      createAddu(DstReg, IntermediateDstReg, SrcReg, Instructions);
 
   } else if ((ImmValue & (0xffffLL << 48)) == 0) {
     if (Is32BitImm) {
       Error(IDLoc, "instruction requires a 32-bit immediate");
       return true;
     }
     warnIfNoMacro(IDLoc);
 
if (0 <= ImmValue && ImmValue <= 65535) {
     //           ori d,d,hi16(lo32(j))
     //           dsll d,d,16
     //           ori d,d,lo16(lo32(j))
     uint16_t Bits47To32 = (ImmValue >> 32) & 0xffff;
     uint16_t Bits31To16 = (ImmValue >> 16) & 0xffff;
     uint16_t Bits15To0 = ImmValue & 0xffff;
 
     tmpInst.setOpcode(Mips::LUi);
-    tmpInst.addOperand(MCOperand::createReg(DstReg));
+    tmpInst.addOperand(MCOperand::createReg(IntermediateDstReg));
     tmpInst.addOperand(MCOperand::createImm(Bits47To32));
     Instructions.push_back(tmpInst);
-    createLShiftOri<0>(Bits31To16, DstReg, IDLoc, Instructions);
-    createLShiftOri<16>(Bits15To0, DstReg, IDLoc, Instructions);
+    createLShiftOri<0>(Bits31To16, IntermediateDstReg, IDLoc, Instructions);
+    createLShiftOri<16>(Bits15To0, IntermediateDstReg, IDLoc, Instructions);
 
     if (UseSrcReg)
-      createAddu(DstReg, DstReg, SrcReg, Instructions);
+      createAddu(DstReg, IntermediateDstReg, SrcReg, Instructions);
 
   } else {
     if (Is32BitImm) {
       Error(IDLoc, "instruction requires a 32-bit immediate");
       return true;
     }
     warnIfNoMacro(IDLoc);
 
if (0 <= ImmValue && ImmValue <= 65535) {
     //           dsll d,d,16
     //           ori d,d,lo16(lo32(j))
     uint16_t Bits63To48 = (ImmValue >> 48) & 0xffff;
     uint16_t Bits47To32 = (ImmValue >> 32) & 0xffff;
     uint16_t Bits31To16 = (ImmValue >> 16) & 0xffff;
     uint16_t Bits15To0 = ImmValue & 0xffff;
 
     tmpInst.setOpcode(Mips::LUi);
-    tmpInst.addOperand(MCOperand::createReg(DstReg));
+    tmpInst.addOperand(MCOperand::createReg(IntermediateDstReg));
     tmpInst.addOperand(MCOperand::createImm(Bits63To48));
     Instructions.push_back(tmpInst);
-    createLShiftOri<0>(Bits47To32, DstReg, IDLoc, Instructions);
+    createLShiftOri<0>(Bits47To32, IntermediateDstReg, IDLoc, Instructions);
 
     // When Bits31To16 is 0, do a left shift of 32 bits instead of doing
     // two left shifts of 16 bits.
     if (Bits31To16 == 0) {
-      createLShiftOri<32>(Bits15To0, DstReg, IDLoc, Instructions);
+      createLShiftOri<32>(Bits15To0, IntermediateDstReg, IDLoc, Instructions);
     } else {
-      createLShiftOri<16>(Bits31To16, DstReg, IDLoc, Instructions);
-      createLShiftOri<16>(Bits15To0, DstReg, IDLoc, Instructions);
+      createLShiftOri<16>(Bits31To16, IntermediateDstReg, IDLoc, Instructions);
+      createLShiftOri<16>(Bits15To0, IntermediateDstReg, IDLoc, Instructions);
     }
 
     if (UseSrcReg)
-      createAddu(DstReg, DstReg, SrcReg, Instructions);
+      createAddu(DstReg, IntermediateDstReg, SrcReg, Instructions);
   }
   return false;
 }
 
 bool MipsAsmParser::expandLoadImm(MCInst &Inst, bool Is32BitImm, SMLoc IDLoc,
                                   SmallVectorImpl<MCInst> &Instructions) {
   const MCOperand &ImmOp = Inst.getOperand(1);
   assert(ImmOp.isImm() && "expected immediate operand kind");

test/MC/Mips/mips-expansions.s

 # CHECK:                                #   fixup A - offset: 0, value: symbol@ABS_LO, kind: fixup_Mips_LO16
 # CHECK: addu $8, $1, $8                # encoding: [0x21,0x40,0x28,0x00]
   la $8, symbol-0x11111($8)
 # CHECK: lui  $1, %hi(symbol-1)         # encoding: [0xff'A',0xff'A',0x01,0x3c]
 # CHECK:                                #   fixup A - offset: 0, value: symbol@ABS_HI, kind: fixup_Mips_HI16
 # CHECK: ori  $1, $1, %lo(symbol-4369)  # encoding: [0xef'A',0xee'A',0x21,0x34]
 # CHECK:                                #   fixup A - offset: 0, value: symbol@ABS_LO, kind: fixup_Mips_LO16
 # CHECK: addu $8, $1, $8                # encoding: [0x21,0x40,0x28,0x00]
+  la $8, 20($8)
+# CHECK: ori  $8, $8, 20                # encoding: [0x14,0x00,0x08,0x35]
+  la $8, 65538($8)
+# CHECK: lui  $1, 1                     # encoding: [0x01,0x00,0x01,0x3c]
+# CHECK: ori  $1, $1, 2                 # encoding: [0x02,0x00,0x21,0x34]
+# CHECK: addu $8, $1, $8                # encoding: [0x21,0x40,0x28,0x00]
 
 # LW/SW and LDC1/SDC1 of symbol address, done by MipsAsmParser::expandMemInst():
   .set noat
   lw $10, symbol($4)
 # CHECK: lui     $10, %hi(symbol)        # encoding: [A,A,0x0a,0x3c]
 # CHECK:                                 #   fixup A - offset: 0, value: symbol@ABS_HI, kind: fixup_Mips_HI16
 # CHECK: addu    $10, $10, $4            # encoding: [0x21,0x50,0x44,0x01]
 # CHECK: lw      $10, %lo(symbol)($10)   # encoding: [A,A,0x4a,0x8d]
 # CHECK:                                 #   fixup A - offset: 0, value: symbol@ABS_LO, kind: fixup_Mips_LO16
   .set at