[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