[PATCH] Implementation of microMIPS 16-bit instructions MOVE and JALR

[Daniel Sanders]

  LGTM with a couple nits.


================
Comment at: lib/Target/Mips/MicroMipsInstrFormats.td:37-40
@@ +36,6 @@
+  field bits<16> SoftFail = 0;
+  bits<6> Opcode = 0x0;
+
+  // Top 6 bits are the 'opcode' field
+  let Inst{15-10} = Opcode;
+}
----------------
This isn't used. Both MOVE_FM_MM16 and JALR_FM_MM16 define Inst and set Inst{15-10} themselves.

If these are removed, then the 'field bits<16> Inst' in this class will become unused.

================
Comment at: lib/Target/Mips/MicroMipsInstrFormats.td:307
@@ -235,3 +306,3 @@
   let Inst{31-26} = 0x00;
   let Inst{25-21} = rd;
   let Inst{20-16} = rs;
----------------
I know this isn't part of your patch, but I noticed that the manual uses 'rt' instead of 'rd' for this field and thought I ought to mention it

================
Comment at: lib/Target/Mips/MicroMipsInstrInfo.td:78
@@ +77,3 @@
+                  !strconcat(opstr, "\t$rd, $rs"),
+                  [(set RO:$rd, (OpNode RO:$rs))], Itin, FrmR> {
+  let isCommutable = isComm;
----------------
I believe you could specify the pattern equivalently as [] and remove the last argument. The only instantiation of this class uses OpNode=null_frag and null_frag causes the pattern to be treated as if [] were given. I think it's slightly easier to see that ISel won't select this instruction if no pattern is specified.

Up to you whether you change it or not because the difference between the null_frag and [] approaches is very small.

================
Comment at: lib/Target/Mips/MipsSEInstrInfo.cpp:91-94
@@ -89,3 +90,6 @@
     if (Mips::GPR32RegClass.contains(SrcReg))
-      Opc = Mips::ADDu, ZeroReg = Mips::ZERO;
+      if (isMicroMips)
+        Opc = Mips::MOVE16_MM;
+      else
+        Opc = Mips::ADDu, ZeroReg = Mips::ZERO;
     else if (Mips::CCRRegClass.contains(SrcReg))
----------------
I'd suggest curly braces around this block


http://llvm-reviews.chandlerc.com/D3112