[PATCH] D9189: [mips][microMIPS] Implement PREFX, LHUE, LBE, LBUE, LHE, LWE, SBE, SHE and SWE instructions

Daniel Sanders via llvm-commits llvm-commits at lists.llvm.org
Fri Sep 4 01:56:45 PDT 2015


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

LGTM with a few nits


================
Comment at: lib/Target/Mips/Disassembler/MipsDisassembler.cpp:281-283
@@ +280,5 @@
+static DecodeStatus DecodeMemMMImm9(MCInst &Inst,
+                                     unsigned Insn,
+                                     uint64_t Address,
+                                     const void *Decoder);
+
----------------
Indentation

================
Comment at: lib/Target/Mips/Disassembler/MipsDisassembler.cpp:1295-1297
@@ +1294,5 @@
+static DecodeStatus DecodeMemMMImm9(MCInst &Inst,
+                                     unsigned Insn,
+                                     uint64_t Address,
+                                     const void *Decoder) {
+  int Offset = SignExtend32<9>(Insn & 0x1ff);
----------------
Indentation

================
Comment at: lib/Target/Mips/MicroMipsInstrFormats.td:392
@@ -391,1 +391,3 @@
 
+class LHUE_FM_MM<bits<6> op, bits<4> fmt, bits<3> funct> : MMArch {
+  bits<5> rt;
----------------
This should start with POOL32C_

================
Comment at: lib/Target/Mips/MicroMipsInstrFormats.td:400-403
@@ +399,6 @@
+  let Inst{25-21} = rt;
+  let Inst{20-16} = addr{20-16};
+  let Inst{15-12} = fmt;
+  let Inst{11-9} = funct;
+  let Inst{8-0}  = addr{8-0};
+}
----------------
Could you assign the RHS to fields (base and offset) first so that the special encoding is self documented?

================
Comment at: lib/Target/Mips/MicroMipsInstrFormats.td:939
@@ -924,1 +938,3 @@
 
+class PREFX_FM_MM<bits<6> op, bits<9> funct> : MMArch {
+  bits<5> index;
----------------
Should begin with POOL32F_


http://reviews.llvm.org/D9189





More information about the llvm-commits mailing list