[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

1740–1741

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

1776–1777

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

151

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

1740–1741

Fixed.

1776–1777

Fixed.

test/MC/Mips/mips-expansions.s

151

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

1748

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

151

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

if (SrcRegOp) {
     SrcReg = SrcRegOp->getReg();
     UseSrcReg = true;
   }
 
   int64_t ImmValue = ImmOp->getImm();
   unsigned DstReg = DstRegOp->getReg();
   MCInst tmpInst;
 
+  bool UseAT = false;
+  unsigned ATReg = 0;
+  if (UseSrcReg && (DstReg == SrcReg)) {
+    UseAT = true;
+    // 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;
+  }
+
   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
     tmpInst.setOpcode(Mips::ORi);
     tmpInst.addOperand(MCOperand::CreateReg(DstReg));
if (0 <= ImmValue && ImmValue <= 65535) {
     tmpInst.addOperand(MCOperand::CreateImm(ImmValue));
     Instructions.push_back(tmpInst);
   } else if ((ImmValue & 0xffffffff) == ImmValue) {
     // For all other values which are representable as a 32-bit integer:
     // li d,j => lui d,hi16(j)
     //           ori d,d,lo16(j)
     uint16_t Bits31To16 = (ImmValue >> 16) & 0xffff;
     uint16_t Bits15To0 = ImmValue & 0xffff;
 
     tmpInst.setOpcode(Mips::LUi);
-    tmpInst.addOperand(MCOperand::CreateReg(DstReg));
+    tmpInst.addOperand(MCOperand::CreateReg(UseAT ? ATReg : DstReg));
     tmpInst.addOperand(MCOperand::CreateImm(Bits31To16));
     Instructions.push_back(tmpInst);
-    createLShiftOri<0>(Bits15To0, DstReg, IDLoc, Instructions);
+    createLShiftOri<0>(Bits15To0, UseAT ? ATReg : DstReg, IDLoc, Instructions);
 
     if (UseSrcReg) {
       tmpInst.clear();
       tmpInst.setOpcode(Mips::ADDu);
       tmpInst.addOperand(MCOperand::CreateReg(DstReg));
-      tmpInst.addOperand(MCOperand::CreateReg(DstReg));
+      tmpInst.addOperand(MCOperand::CreateReg(UseAT ? ATReg : DstReg));
       tmpInst.addOperand(MCOperand::CreateReg(SrcReg));
       Instructions.push_back(tmpInst);
     }
   } else if ((ImmValue & (0xffffLL << 48)) == 0) {
     if (Is32BitImm) {
       Error(IDLoc, "instruction requires a 32-bit immediate");
       return true;
     }
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(UseAT ? ATReg : DstReg));
     tmpInst.addOperand(MCOperand::CreateImm(Bits47To32));
     Instructions.push_back(tmpInst);
-    createLShiftOri<0>(Bits31To16, DstReg, IDLoc, Instructions);
-    createLShiftOri<16>(Bits15To0, DstReg, IDLoc, Instructions);
+    createLShiftOri<0>(Bits31To16, UseAT ? ATReg : DstReg, IDLoc, Instructions);
+    createLShiftOri<16>(Bits15To0, UseAT ? ATReg : DstReg, IDLoc, Instructions);
 
     if (UseSrcReg) {
       tmpInst.clear();
       tmpInst.setOpcode(Mips::ADDu);
       tmpInst.addOperand(MCOperand::CreateReg(DstReg));
-      tmpInst.addOperand(MCOperand::CreateReg(DstReg));
+      tmpInst.addOperand(MCOperand::CreateReg(UseAT ? ATReg : DstReg));
       tmpInst.addOperand(MCOperand::CreateReg(SrcReg));
       Instructions.push_back(tmpInst);
     }
   } else {
     if (Is32BitImm) {
       Error(IDLoc, "instruction requires a 32-bit immediate");
       return true;
     }
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(UseAT ? ATReg : DstReg));
     tmpInst.addOperand(MCOperand::CreateImm(Bits63To48));
     Instructions.push_back(tmpInst);
-    createLShiftOri<0>(Bits47To32, DstReg, IDLoc, Instructions);
+    createLShiftOri<0>(Bits47To32, UseAT ? ATReg : DstReg, 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>(0, DstReg, IDLoc, Instructions);
-      createLShiftOri<0>(Bits15To0, DstReg, IDLoc, Instructions);
+      createLShiftOri<32>(0, UseAT ? ATReg : DstReg, IDLoc, Instructions);
+      createLShiftOri<0>(Bits15To0, UseAT ? ATReg : DstReg, IDLoc, Instructions);
     } else {
-      createLShiftOri<16>(Bits31To16, DstReg, IDLoc, Instructions);
-      createLShiftOri<16>(Bits15To0, DstReg, IDLoc, Instructions);
+      createLShiftOri<16>(Bits31To16, UseAT ? ATReg : DstReg, IDLoc, Instructions);
+      createLShiftOri<16>(Bits15To0, UseAT ? ATReg : DstReg, IDLoc, Instructions);
     }
 
     if (UseSrcReg) {
       tmpInst.clear();
       tmpInst.setOpcode(Mips::ADDu);
       tmpInst.addOperand(MCOperand::CreateReg(DstReg));
-      tmpInst.addOperand(MCOperand::CreateReg(DstReg));
+      tmpInst.addOperand(MCOperand::CreateReg(UseAT ? ATReg : DstReg));
       tmpInst.addOperand(MCOperand::CreateReg(SrcReg));
       Instructions.push_back(tmpInst);
     }
   }
   return false;
 }
 
 bool

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]
 # 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]
 
+# CHECK: ori  $8, $8, 20                # encoding: [0x14,0x00,0x08,0x35]
+# 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]
+
 # 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
 # CHECK: lui     $1, %hi(symbol)         # encoding: [A,A,0x01,0x3c]
 # CHECK:                                 #   fixup A - offset: 0, value: symbol@ABS_HI, kind: fixup_Mips_HI16
 # CHECK: addu    $1, $1, $9              # encoding: [0x21,0x08,0x29,0x00]
# CHECK: sdc1    $f0, %lo(symbol)($1)
     la $8, symbol+32770($8)
     la $8, symbol+65538($8)
     la $8, symbol+0x11111($8)
     la $8, symbol-1($8)
     la $8, symbol-32770($8)
     la $8, symbol-65538($8)
     la $8, symbol-0x11111($8)
 
+    la $8, 20($8)
+    la $8, 65538($8)
+
     .set noat
     lw  $t2, symbol($a0)
     .set at
     sw  $t2, symbol($t1)
 
     lw  $t2, 655483($a0)
     sw  $t2, 123456($t1)
 
     lw  $8, symbol
     sw  $8, symbol
 
     ldc1 $f0, symbol
     sdc1 $f0, symbol