[PATCH] D18640: [mips][microMIPS]Implement CFC*, CTC* and LDC* instructions

[Simon Dardis via llvm-commits]

sdardis requested changes to this revision.
sdardis added a comment.
This revision now requires changes to proceed.

Minor nits.

For the (in)valid tests, can you integrate them alphabetically into the corresponding instruction groups.

Can you rebase this to ToT?


================
Comment at: lib/Target/Mips/MicroMipsInstrFPU.td:150
@@ +149,3 @@
+  def LDC1_MM : MMRel, LW_FT<"ldc1", AFGR64Opnd, II_LDC1, load>,
+                LW_FM_MM<0x2f>, FGR_32{
+    let DecoderNamespace = "MicroMips";
----------------
Space between the FGR_32 and the {.

================
Comment at: lib/Target/Mips/MipsInstrInfo.td:1088
@@ -1087,3 +1087,3 @@
 // COP2 Load/Store
-class LW_FT2<string opstr, RegisterOperand RC, InstrItinClass Itin,
+class LW_FT2<string opstr, RegisterOperand RC, Operand MemOpnd, InstrItinClass Itin,
              SDPatternOperator OpNode= null_frag> :
----------------
Formatting, line length should not exceed 80 characters if possible. Start a newline after the new parameter (Operand MemOpnd).

================
Comment at: lib/Target/Mips/MipsInstrInfo.td:1647
@@ -1646,3 +1646,3 @@
            ISA_MIPS1_NOT_32R6_64R6;
-def LDC2 : LW_FT2<"ldc2", COP2Opnd, NoItinerary, load>, LW_FM<0x36>,
+def LDC2 : StdMMR6Rel, LW_FT2<"ldc2", COP2Opnd, mem_simm16, NoItinerary, load>, LW_FM<0x36>,
            ISA_MIPS2_NOT_32R6_64R6;
----------------
Formatting, line length should not exceed 80 characters. Start a new line after "load>,".

================
Comment at: test/MC/Mips/mips32r6/invalid.s:16
@@ -15,2 +15,3 @@
         ldc2    $8,-21181($at)   # CHECK: :[[@LINE]]:{{[0-9]+}}: error: instruction requires a CPU feature not currently enabled
+        ldc2    $11, 1024($12)   # CHECK: :[[@LINE]]:{{[0-9]+}}: error: instruction requires a CPU feature not currently enabled
         sdc2    $20,23157($s2)   # CHECK: :[[@LINE]]:{{[0-9]+}}: error: instruction requires a CPU feature not currently enabled
----------------
What is being tested here that line 15 does not cover?

================
Comment at: test/MC/Mips/mips64r6/invalid.s:16
@@ -15,2 +15,3 @@
         ldc2    $8,-21181($at)   # CHECK: :[[@LINE]]:{{[0-9]+}}: error: instruction requires a CPU feature not currently enabled
+        ldc2    $11, 1024($12)   # CHECK: :[[@LINE]]:{{[0-9]+}}: error: instruction requires a CPU feature not currently enabled
         break -1          # CHECK: :[[@LINE]]:15: error: expected 10-bit unsigned immediate
----------------
See my comment on mips32r6/invalid.s.


http://reviews.llvm.org/D18640