[PATCH] D15570: [mips][microMIPS] Implement DERET and DI instructions and check size operand for EXT and DEXT* instructions

[Daniel Sanders via llvm-commits]

dsanders accepted this revision.
dsanders added a comment.
This revision is now accepted and ready to land.

Thanks for adding some checking to the size operand of ext.

LGTM with a couple nits


================
Comment at: lib/Target/Mips/MicroMips32r6InstrInfo.td:55-56
@@ -54,3 +54,4 @@
 class EHB_MMR6_ENC : BARRIER_MMR6_ENC<"ehb", 0x3>;
 class EI_MMR6_ENC : EIDI_MMR6_ENC<"ei", 0x15d>;
-class ERET_MMR6_ENC : ERET_FM_MMR6<"eret">;
+class DI_MMR6_ENC : EIDI_MMR6_ENC<"di", 0b0100011101>;
+class ERET_MMR6_ENC : POOL32A_ERET_FM_MMR6<"eret", 0x3cd>;
----------------
These two are in POOL32A. I must have missed this when EI was added.

================
Comment at: lib/Target/Mips/MicroMips64r6InstrInfo.td:68-73
@@ -66,8 +67,8 @@
 }
-class DEXT_MMR6_DESC : EXTBITS_DESC_BASE<"dext", GPR64Opnd, uimm6,
-                                                 MipsExt>;
+class DEXT_MMR6_DESC : EXTBITS_DESC_BASE<"dext", GPR64Opnd, uimm5,
+                                         uimm5_plus1, MipsExt>;
 class DEXTM_MMR6_DESC : EXTBITS_DESC_BASE<"dextm", GPR64Opnd, uimm5,
-                                                  MipsExt>;
+                                          uimm5_plus33, MipsExt>;
 class DEXTU_MMR6_DESC : EXTBITS_DESC_BASE<"dextu", GPR64Opnd, uimm5_plus32,
-                                                  MipsExt>;
+                                          uimm5_plus1, MipsExt>;
 
----------------
Could you mention that we don't check the '0 < pos + size <= 63' constraint yet?

================
Comment at: test/MC/Mips/micromips64r6/invalid.s:21
@@ -20,3 +20,1 @@
   cache 32, 255($7)        # CHECK: :[[@LINE]]:9: error: expected 5-bit unsigned immediate
-  # FIXME: Check size on dext*
-  dext $2, $3, -1, 1   # CHECK: :[[@LINE]]:16: error: expected 6-bit unsigned immediate
----------------
We haven't completely fixed this yet since the valid range for the size is also influenced by the value used for pos. We still need the FIXME but it can be more specific now.

Likewise for the other instances of this FIXME.


http://reviews.llvm.org/D15570