[PATCH] D10337: [mips][microMIPS] Implement CACHEE, WRPGPR and WSBH instructions

[Daniel Sanders via llvm-commits]

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

LGTM with some nits.


================
Comment at: lib/Target/Mips/Disassembler/MipsDisassembler.cpp:1109-1117
@@ -1108,2 +1108,11 @@
 
+  switch (Inst.getOpcode()) {
+    default:
+      Offset = SignExtend32<12>(Insn & 0xfff);
+      break;
+    case Mips::CACHEE_MMR6:
+      Offset = SignExtend32<9>(Insn & 0x1ff);
+      break;
+  }
+
   Inst.addOperand(MCOperand::createReg(Base));
----------------
Indentation (labels are the same indentation level as the switch in LLVM's style).

================
Comment at: lib/Target/Mips/MicroMips32r6InstrFormats.td:47
@@ +46,3 @@
+class CACHEE_FM_MMR6<bits<6> opgroup, bits<4> fmt, bits<3> funct> : MipsR6Inst {
+  bits<21> addr;
+  bits<5> hint;
----------------
As per the other reviews, please assign these to fields named 'base' and 'offset' to make the special encoding self-documenting.

  bits<21> addr;
  bits<5> base = addr{20-16};
  bits<9> offset = addr{8-0};

  ...
  let Inst{20-16} = base;
  ...
  let Inst{8-0} = offset;

================
Comment at: lib/Target/Mips/MicroMips32r6InstrInfo.td:263
@@ +262,3 @@
+}
+class WRPGPR_MMR6_DESC : WORDSWAP_MMR6_DESC_BASE<"wrpgpr", GPR32Opnd>;
+class WSBH_MMR6_DESC : WORDSWAP_MMR6_DESC_BASE<"wsbh", GPR32Opnd>;
----------------
wrpgpr isn't a 'word swap', it's a register move.

================
Comment at: lib/Target/Mips/MicroMips32r6InstrInfo.td:264
@@ +263,3 @@
+class WRPGPR_MMR6_DESC : WORDSWAP_MMR6_DESC_BASE<"wrpgpr", GPR32Opnd>;
+class WSBH_MMR6_DESC : WORDSWAP_MMR6_DESC_BASE<"wsbh", GPR32Opnd>;
+
----------------
'WORDSWAP' is misleading. WSBH swaps bytes within each half-word (1234 -> 2143).


http://reviews.llvm.org/D10337