[PATCH] D12628: [mips][microMIPS] Implement PAUSE, RDHWR, RDPGPR, SDBBP, SIGRIE, SSNOP, SYNC, SYNCI and WAIT instructions
Daniel Sanders via llvm-commits
llvm-commits at lists.llvm.org
Wed Sep 16 05:36:04 PDT 2015
dsanders requested changes to this revision.
This revision now requires changes to proceed.
================
Comment at: lib/Target/Mips/Disassembler/MipsDisassembler.cpp:265-267
@@ +264,5 @@
+static DecodeStatus DecodeSynciR6(MCInst &Inst,
+ unsigned Insn,
+ uint64_t Address,
+ const void *Decoder);
+
----------------
Indentation
================
Comment at: lib/Target/Mips/Disassembler/MipsDisassembler.cpp:1172-1174
@@ +1171,5 @@
+static DecodeStatus DecodeSynciR6(MCInst &Inst,
+ unsigned Insn,
+ uint64_t Address,
+ const void *Decoder) {
+ int Immediate = SignExtend32<16>(Insn & 0xffff);
----------------
Indentation
================
Comment at: lib/Target/Mips/MicroMips32r6InstrFormats.td:127-200
@@ -126,2 +126,76 @@
+class PAUSE_FM_MMR6<string instr_asm, bits<5> op> : MMR6Arch<instr_asm> {
+ bits<32> Inst;
+
+ let Inst{31-26} = 0;
+ let Inst{25-21} = 0;
+ let Inst{20-16} = 0;
+ let Inst{15-11} = op;
+ let Inst{10-6} = 0;
+ let Inst{5-0} = 0;
+}
+
+class READ_REGISTER_FM_MMR6<bits<10> funct> {
+ bits<5> rt;
+ bits<5> rd;
+ bits<32> Inst;
+
+ let Inst{31-26} = 0;
+ let Inst{25-21} = rt;
+ let Inst{20-16} = rd;
+ let Inst{15-6} = funct;
+ let Inst{5-0} = 0b111100;
+}
+
+class POOL32A_RDHWR_FM_MMR6 {
+ bits<5> rt;
+ bits<5> rs;
+ bits<3> sel;
+ bits<32> Inst;
+
+ let Inst{31-26} = 0;
+ let Inst{25-21} = rt;
+ let Inst{20-16} = rs;
+ let Inst{15-14} = 0;
+ let Inst{13-11} = sel;
+ let Inst{10} = 0;
+ let Inst{9-0} = 0b0111000000;
+}
+
+class SYNC_FM_MMR6 {
+ bits<5> stype;
+
+ bits<32> Inst;
+
+ let Inst{31-26} = 0;
+ let Inst{25-21} = 0;
+ let Inst{20-16} = stype;
+ let Inst{15-6} = 0b0110101101;
+ let Inst{5-0} = 0b111100;
+}
+
+class SYNCI_FM_MMR6 {
+ bits<21> addr;
+ bits<5> base = addr{20-16};
+ bits<16> immediate = addr{15-0};
+
+ bits<32> Inst;
+
+ let Inst{31-26} = 0b010000;
+ let Inst{25-21} = 0b01100;
+ let Inst{20-16} = base;
+ let Inst{15-0} = immediate;
+}
+
+class SDBBP_FM_MMR6 {
+ bits<10> code_;
+
+ bits<32> Inst;
+
+ let Inst{31-26} = 0;
+ let Inst{25-16} = code_;
+ let Inst{15-6} = 0b1101101101;
+ let Inst{5-0} = 0b111100;
+}
+
class POOL32A_2R_FM_MMR6<bits<10> funct> : MipsR6Inst {
----------------
All but one of these need to be renamed to follow the convention of <major-opcode-name>_<string>_FM_MMR6.
POOL32A_RDHWR_FM_MMR6 is correct.
================
Comment at: lib/Target/Mips/MicroMips32r6InstrInfo.td:298-308
@@ +297,13 @@
+class PAUSE_MMR6_DESC : Barrier<"pause">;
+class RDHWR_MMR6_DESC_BASE<string instr_asm, RegisterOperand CPURegOperand,
+ RegisterOperand RO, Operand ImmOpnd> : MMR6Arch<"rdhwr">, MipsR6Inst {
+ dag OutOperandList = (outs CPURegOperand:$rt);
+ dag InOperandList = (ins RO:$rs, ImmOpnd:$sel);
+ string AsmString = !strconcat(instr_asm, "\t$rt, $rs, $sel");
+ list<dag> Pattern = [];
+ InstrItinClass Itinerary = II_RDHWR;
+ Format Form = FrmR;
+}
+class RDHWR_MMR6_DESC : RDHWR_MMR6_DESC_BASE<"rdhwr", GPR32Opnd, HWRegsOpnd,
+ uimm3>;
+class WAIT_MMR6_DESC : WaitMM<"wait">;
----------------
You don't need separate a *_DESC_BASE class if there's only one subclass.
================
Comment at: lib/Target/Mips/MicroMips32r6InstrInfo.td:299
@@ +298,3 @@
+class RDHWR_MMR6_DESC_BASE<string instr_asm, RegisterOperand CPURegOperand,
+ RegisterOperand RO, Operand ImmOpnd> : MMR6Arch<"rdhwr">, MipsR6Inst {
+ dag OutOperandList = (outs CPURegOperand:$rt);
----------------
Indentation
================
Comment at: lib/Target/Mips/MicroMips32r6InstrInfo.td:308
@@ +307,3 @@
+class RDHWR_MMR6_DESC : RDHWR_MMR6_DESC_BASE<"rdhwr", GPR32Opnd, HWRegsOpnd,
+ uimm3>;
+class WAIT_MMR6_DESC : WaitMM<"wait">;
----------------
Indentation
================
Comment at: lib/Target/Mips/MicroMips32r6InstrInfo.td:336-365
@@ -313,2 +335,32 @@
+class SYNC_MMR6_DESC_BASE<string instr_asm> : MMR6Arch<instr_asm>, MipsR6Inst {
+ dag OutOperandList = (outs);
+ dag InOperandList = (ins i32imm:$stype);
+ string AsmString = !strconcat(instr_asm, "\t$stype");
+ list<dag> Pattern = [(MipsSync imm:$stype)];
+ InstrItinClass Itinerary = NoItinerary;
+ bit HasSideEffects = 1;
+}
+class SYNC_MMR6_DESC : SYNC_MMR6_DESC_BASE<"sync">;
+
+class SYNCI_MMR6_DESC_BASE<string instr_asm> : SYNCI_FT<instr_asm> {
+ let DecoderMethod = "DecodeSynciR6";
+}
+class SYNCI_MMR6_DESC : SYNCI_MMR6_DESC_BASE<"synci">;
+
+class RDPGPR_MMR6_DESC_BASE<string instr_asm, RegisterOperand GPROpnd> : MMR6Arch<instr_asm>, MipsR6Inst {
+ dag OutOperandList = (outs GPROpnd:$rt);
+ dag InOperandList = (ins GPROpnd:$rd);
+ string AsmString = !strconcat(instr_asm, "\t$rt, $rd");
+}
+class RDPGPR_MMR6_DESC : RDPGPR_MMR6_DESC_BASE<"rdpgpr", GPR32Opnd>;
+
+class SDBBP_MMR6_DESC_BASE<string instr_asm> : MipsR6Inst {
+ dag OutOperandList = (outs);
+ dag InOperandList = (ins uimm20:$code_);
+ string AsmString = !strconcat(instr_asm, "\t$code_");
+ list<dag> Pattern = [];
+}
+class SDBBP_MMR6_DESC : SDBBP_MMR6_DESC_BASE<"sdbbp">;
+
//===----------------------------------------------------------------------===//
----------------
You don't need a separate *_DESC_BASE class if there's only one subclass.
================
Comment at: lib/Target/Mips/MicroMips32r6InstrInfo.td:351
@@ +350,3 @@
+
+class RDPGPR_MMR6_DESC_BASE<string instr_asm, RegisterOperand GPROpnd> : MMR6Arch<instr_asm>, MipsR6Inst {
+ dag OutOperandList = (outs GPROpnd:$rt);
----------------
Is this under 80 cols?
================
Comment at: lib/Target/Mips/MicroMips32r6InstrInfo.td:464
@@ +463,2 @@
+def : MipsInstAlias<"rdhwr $rt, $rs",
+ (RDHWR_MMR6 GPR32Opnd:$rt, HWRegsOpnd:$rs, 0), 1>, ISA_MICROMIPS32R6;
----------------
Indentation
================
Comment at: lib/Target/Mips/MicroMipsInstrInfo.td:870
@@ -869,4 +869,3 @@
def TLBWI_MM : MMRel, TLB<"tlbwi">, COP0_TLB_FM_MM<0x8d>;
- def TLBWR_MM : MMRel, TLB<"tlbwr">, COP0_TLB_FM_MM<0xcd>;
-
+ def TLBWR_MM : MMRel, StdMMR6Rel, TLB<"tlbwr">, COP0_TLB_FM_MM<0xcd>;
def SDBBP_MM : MMRel, SYS_FT<"sdbbp">, SDBBP_FM_MM;
----------------
Why is this changed? You don't mention TLBWR in your commit message.
================
Comment at: lib/Target/Mips/MicroMipsInstrInfo.td:876
@@ -874,1 +875,3 @@
+ def RDHWR_MM : MMRel, R6MMR6Rel, ReadHardware<GPR32Opnd, HWRegsOpnd>,
+ RDHWR_FM_MM, ISA_MICROMIPS32R2;
}
----------------
Indentation
================
Comment at: test/MC/Mips/micromips-control-instructions.s:14
@@ -13,6 +13,3 @@
# CHECK-EL: sdbbp 34 # encoding: [0x22,0x00,0x7c,0xdb]
-# CHECK-EL: .set push
-# CHECK-EL: .set mips32r2
-# CHECK-EL: rdhwr $5, $29
-# CHECK-EL: .set pop # encoding: [0xbd,0x00,0x3c,0x6b]
+# CHECK-EL: rdhwr $5, $29 # encoding: [0xbd,0x00,0x3c,0x6b]
# CHECK-EL: cache 1, 8($5) # encoding: [0x25,0x20,0x08,0x60]
----------------
Have you fixed the emission of the .set's or have you only removed the checks?
If it's the former, can you point me at the fix? I don't think I've seen it.
If it's the latter, I'd rather leave them in and turn them into CHECK-NOT's when we fix it.
http://reviews.llvm.org/D12628
More information about the llvm-commits
mailing list