[PATCH] D18824: [mips][microMIPS] Implement LWC1, LWC2, SDC1, SDC2, SWC1 and SWC2 instructions

Daniel Sanders via llvm-commits llvm-commits at lists.llvm.org
Fri Apr 8 08:58:00 PDT 2016


dsanders requested changes to this revision.
dsanders added a comment.
This revision now requires changes to proceed.

Could you add some codegen test cases for the new instructions?

By the way, was leaving LDC1/LDC2 out of this patch intentional?


================
Comment at: lib/Target/Mips/Disassembler/MipsDisassembler.cpp:1599-1614
@@ -1590,2 +1598,18 @@
 
+static DecodeStatus DecodeFMemMMR6(MCInst &Inst, unsigned Insn,
+                                   uint64_t Address, const void *Decoder) {
+  int Offset = SignExtend32<16>(Insn & 0xffff);
+  unsigned Base = fieldFromInstruction(Insn, 16, 5);
+  unsigned Reg = fieldFromInstruction(Insn, 21, 5);
+
+  Reg = getReg(Decoder, Mips::FGR64RegClassID, Reg);
+  Base = getReg(Decoder, Mips::GPR32RegClassID, Base);
+
+  Inst.addOperand(MCOperand::createReg(Reg));
+  Inst.addOperand(MCOperand::createReg(Base));
+  Inst.addOperand(MCOperand::createImm(Offset));
+
+  return MCDisassembler::Success;
+}
+
 static DecodeStatus DecodeFMem2(MCInst &Inst,
----------------
This function is the same as DecodeFMem but with the Reg and Base fields swapped. Could you add a comment saying that this is intentional and correct?

================
Comment at: lib/Target/Mips/MicroMips32r6InstrInfo.td:660
@@ +659,3 @@
+  string AsmString = !strconcat(opstr, "\t$rt, $addr");
+  list<dag> Pattern = [(set COPOpnd:$rt, (OpNode addrDefault:$addr))];
+  Format f = FrmFI;
----------------
addrDefault does not permit offsets but the instruction supports 11-bit. See addrimm12 for an example that supports 12-bit offsets.

================
Comment at: lib/Target/Mips/MicroMips32r6InstrInfo.td:674
@@ +673,3 @@
+  string AsmString = !strconcat("sdc1", "\t$ft, $addr");
+  list<dag> Pattern = [(store FGR64Opnd:$ft, addrDefault:$addr)];
+  Format f = FrmFI;
----------------
addrDefault -> addr to support the 16-bit offset the instruction allows

================
Comment at: lib/Target/Mips/MicroMips32r6InstrInfo.td:688
@@ +687,3 @@
+  string AsmString = !strconcat(opstr, "\t$rt, $addr");
+  list<dag> Pattern = [(OpNode COPOpnd:$rt, addrDefault:$addr)];
+  Format f = FrmFI;
----------------
As per the LWC2 case, we should use an 11-bit version of addrimm12 to support the 11-bit offset the instruction allows

================
Comment at: lib/Target/Mips/MicroMipsInstrFPU.td:145-152
@@ -149,2 +144,10 @@
                  MFC1_FM_MM<0xc0>, ISA_MIPS32R2, FGR_32;
+  let DecoderNamespace = "MicroMips",  DecoderMethod = "DecodeFMemMMR6" in {
+    def LWC1_MM : MMRel, LW_FT<"lwc1", FGR32Opnd, II_LWC1, load>,
+                  LW_FM_MM<0x27>;
+    def SDC1_MM : MMRel, SW_FT<"sdc1", AFGR64Opnd, II_SDC1, store>,
+                  LW_FM_MM<0x2e>, FGR_32;
+    def SWC1_MM : MMRel, SW_FT<"swc1", FGR32Opnd, II_SWC1, store>,
+                  LW_FM_MM<0x26>;
+  }
 }
----------------
The DecodeNamespace and Predicates/AdditionalPredicates change makes sense but the DecoderMethod change looks wrong. It's applying microMIPSR6 specific decoder methods to microMIPSR2 instructions.


http://reviews.llvm.org/D18824





More information about the llvm-commits mailing list