[PATCH] D11801: [mips][microMIPS] Implement SB, SBE, SCE, SH and SHE 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 a few nits


================
Comment at: lib/Target/Mips/Disassembler/MipsDisassembler.cpp:260-262
@@ +259,5 @@
+static DecodeStatus DecodeStoreEvaOpMM(MCInst &Inst,
+                                    unsigned Insn,
+                                    uint64_t Address,
+                                    const void *Decoder);
+
----------------
Nit: Indentation

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

================
Comment at: lib/Target/Mips/MCTargetDesc/MipsMCCodeEmitter.cpp:771
@@ -770,1 +770,3 @@
 
+ unsigned MipsMCCodeEmitter::
+getMemEncodingMMImm9(const MCInst &MI, unsigned OpNo,
----------------
Nit: Indentation

================
Comment at: lib/Target/Mips/MCTargetDesc/MipsMCCodeEmitter.cpp:773-774
@@ +772,4 @@
+getMemEncodingMMImm9(const MCInst &MI, unsigned OpNo,
+                      SmallVectorImpl<MCFixup> &Fixups,
+                      const MCSubtargetInfo &STI) const {
+  // Base register is encoded in bits 20-16, offset is encoded in bits 8-0.
----------------
Nit: Indentation

================
Comment at: lib/Target/Mips/MCTargetDesc/MipsMCCodeEmitter.h:176-177
@@ -175,1 +175,4 @@
+  unsigned getMemEncodingMMImm9(const MCInst &MI, unsigned OpNo,
+                                 SmallVectorImpl<MCFixup> &Fixups,
+                                 const MCSubtargetInfo &STI) const;
   unsigned getMemEncodingMMImm12(const MCInst &MI, unsigned OpNo,
----------------
Nit: Indentation

================
Comment at: lib/Target/Mips/MicroMips32r6InstrFormats.td:201-226
@@ -200,2 +200,28 @@
 
+class STORE_FM_MMR6<bits<6> op> {
+  bits<5> rt;
+  bits<21> addr;
+
+  bits<32> Inst;
+
+  let Inst{31-26} = op;
+  let Inst{25-21} = rt;
+  let Inst{20-16} = addr{20-16};
+  let Inst{15-0}  = addr{15-0};
+}
+
+class STORE_EVA_FM_MMR6<bits<3> funct> {
+  bits<5> rt;
+  bits<21> addr;
+
+  bits<32> Inst;
+
+  let Inst{31-26} = 0b011000;
+  let Inst{25-21} = rt;
+  let Inst{20-16} = addr{20-16}; // base
+  let Inst{15-12} = 0b1010;
+  let Inst{11-9} = funct;
+  let Inst{8-0}  = addr{8-0}; // offset
+}
+
 class CMP_BRANCH_1R_RT_OFF16_FM_MMR6<bits<6> funct> : MipsR6Inst {
----------------
Please break out the fields of addr using variables.

For example:
  bits<21> addr;
  let Inst{20-16} = addr{20-16};
  let Inst{15-0}  = addr{15-0};
should be something like:
  bits<21> addr;
  bits<5> base = addr{20-16};
  bits<16> offset = addr{15-0};
  let Inst{20-16} = base;
  let Inst{15-0}  = offset;

This helps to document the unusual encoding we use for addr fields.

================
Comment at: lib/Target/Mips/MicroMips32r6InstrInfo.td:122-129
@@ -116,2 +121,10 @@
 
+def mem_mm_9 : Operand<i32> {
+  let PrintMethod = "printMemOperand";
+  let MIOperandInfo = (ops GPR32, simm9);
+  let EncoderMethod = "getMemEncodingMMImm9";
+  let ParserMatchClass = MipsMemAsmOperand;
+  let OperandType = "OPERAND_MEMORY";
+}
+
 class ADD_MMR6_DESC : ArithLogicR<"add", GPR32Opnd>;
----------------
This belongs at the top of the file, before the *_ENC classes.

================
Comment at: lib/Target/Mips/MicroMips32r6InstrInfo.td:294
@@ +293,3 @@
+class STORE_MMR6_DESC_BASE<string opstr, DAGOperand RO> :
+          Store<opstr, RO>, MMR6Arch<opstr> {
+  let DecoderMethod = "DecodeMemMMImm16";
----------------
Nit: Indentation

================
Comment at: lib/Target/Mips/MicroMips32r6InstrInfo.td:300
@@ +299,3 @@
+class STORE_EVA_MMR6_DESC_BASE<string instr_asm, RegisterOperand RO> :
+        MMR6Arch<instr_asm>, MipsR6Inst {
+  dag OutOperandList = (outs);
----------------
Nit: Indentation


http://reviews.llvm.org/D11801