[PATCH] [mips][microMIPS] Implement LWXS, LWP and SWP instructions

[Daniel Sanders]

================
Comment at: lib/Target/Mips/AsmParser/MipsAsmParser.cpp:3449-3454
@@ -3444,2 +3448,8 @@
   MipsOperand &Op = static_cast<MipsOperand &>(*Operands.back());
+
+  if (!Op.isGPRAsmReg()) {
+    Error(E, "invalid operand for instruction");
+    return MatchOperand_ParseFail;
+  }
+
   unsigned Reg = Op.getGPR32Reg();
----------------
While this looks like a sensible thing to do I'm afraid this isn't correct. The problems it causes will be very subtle and might not reveal themselves for a long time. It's not a parse failure to let non-GPR's through here, we just have to catch them during matching using predicates instead.

The root of the problem is that all parsing decisions are final since there is no backtracking. Making matching decisions during the parse can can cause erratic behaviour that depends on the order of the matcher table. About a year ago, this became apparent when the parser was selecting FPU register classes at parse time. As a result, the matcher was failing to match legitimate code because the parser had made the wrong decision, blocking the possibility of matching the correct instruction. This is also why numeric registers aren't resolved to a register class until the moment they are rendered into an instruction. If we decided earlier, then an incorrect decision would prevent the assembler matching valid instructions.

================
Comment at: lib/Target/Mips/Disassembler/MipsDisassembler.cpp:1326-1328
@@ -1325,3 +1325,5 @@
     Inst.addOperand(MCOperand::createReg(Reg));
-    if (Inst.getOpcode() == Mips::LWP_MM || Inst.getOpcode() == Mips::SWP_MM)
+    if (Inst.getOpcode() == Mips::LWP_MM || Inst.getOpcode() == Mips::SWP_MM
+      || Inst.getOpcode() == Mips::LWP_MMR6
+      || Inst.getOpcode() == Mips::SWP_MMR6)
       Inst.addOperand(MCOperand::createReg(Reg+1));
----------------
Nit: LLVM convention is operators on the previous line like so:
  if (Inst.getOpcode() == Mips::LWP_MM || Inst.getOpcode() == Mips::SWP_MM ||
      Inst.getOpcode() == Mips::LWP_MMR6 || Inst.getOpcode() == Mips::SWP_MMR6)

================
Comment at: lib/Target/Mips/MicroMips32r6InstrFormats.td:252-254
@@ +251,5 @@
+  let Inst{25-21} = rd;
+  let Inst{20-16} = addr{20-16};
+  let Inst{15-12} = funct;
+  let Inst{11-0}  = addr{11-0};
+}
----------------
Please extract the two pieces of addr to separate fields as per CACHE_PREF_FM_MM. It serves to explain the unusual encoding.

================
Comment at: lib/Target/Mips/MicroMips32r6InstrInfo.td:250-261
@@ -247,1 +249,14 @@
 
+class LWXS_MMR6_DESC_BASE<string instr_asm, RegisterOperand RO>
+    : MMR6Arch<instr_asm> {
+  dag OutOperandList = (outs RO:$rd);
+  dag InOperandList = (ins PtrRC:$base, PtrRC:$index);
+  string AsmString = !strconcat(instr_asm, "\t$rd, ${index}(${base})");
+  list<dag> Pattern = [];
+  InstrItinClass Itin = NoItinerary;
+  SDPatternOperator OpNode = null_frag;
+  Format f = FrmFI;
+  string BaseOpcode = instr_asm;
+}
+class LWXS_MMR6_DESC : LWXS_MMR6_DESC_BASE<"lwxs", GPR32Opnd>;
+
----------------
LWXS was removed in microMIPSr6

================
Comment at: lib/Target/Mips/MicroMips32r6InstrInfo.td:263-292
@@ -248,1 +262,32 @@
+
+class LWP_MMR6_DESC_BASE<string instr_asm>
+    : MMR6Arch<instr_asm> {
+  dag OutOperandList = (outs regpair:$rd);
+  dag InOperandList = (ins mem_simm12gpr:$addr);
+  string AsmString = !strconcat(instr_asm, "\t$rd, $addr");
+  list<dag> Pattern = [];
+  InstrItinClass Itin = NoItinerary;
+  ComplexPattern Addr = addr;
+  Format f = FrmI;
+  string BaseOpcode = instr_asm;
+  string DecoderMethod = "DecodeMemMMImm12";
+  bit mayLoad = 1;
+}
+class LWP_MMR6_DESC : LWP_MMR6_DESC_BASE<"lwp">;
+
+class SWP_MMR6_DESC_BASE<string instr_asm>
+    : MMR6Arch<instr_asm> {
+  dag OutOperandList = (outs);
+  dag InOperandList = (ins regpair:$rd, mem_simm12gpr:$addr);
+  string AsmString = !strconcat(instr_asm, "\t$rd, $addr");
+  list<dag> Pattern = [];
+  InstrItinClass Itin = NoItinerary;
+  ComplexPattern Addr = addr;
+  Format f = FrmI;
+  string BaseOpcode = instr_asm;
+  string DecoderMethod = "DecodeMemMMImm12";
+  bit mayStore = 1;
+}
+class SWP_MMR6_DESC : SWP_MMR6_DESC_BASE<"swp">;
+
 class SELEQNE_Z_MMR6_DESC_BASE<string instr_asm, RegisterOperand GPROpnd>
----------------
Possible Nit: There's no need for a separate *_DESC_BASE if there's only one *_DESC inheriting from it. Are there more instructions on the way that will use these *_DESC_BASE's?

http://reviews.llvm.org/D10640

EMAIL PREFERENCES
  http://reviews.llvm.org/settings/panel/emailpreferences/