[PATCH] D18645: [mips][microMIPS] Fix offsets for LLE, LWE, SBE, SCE and SHE instructions
Daniel Sanders via llvm-commits
llvm-commits at lists.llvm.org
Thu Mar 31 07:07:35 PDT 2016
dsanders requested changes to this revision.
dsanders added a comment.
This revision now requires changes to proceed.
Hi,
There's probably some overlap between this patch and the http://reviews.llvm.org/rL265010 I committed earlier today. You seem to have spotted some bugs that I missed though (the changes from 12-bit offsets to 9-bit offsets).
================
Comment at: lib/Target/Mips/MicroMips32r6InstrFormats.td:394-422
@@ -393,31 +393,31 @@
-class POOL32C_STORE_EVA_FM_MMR6<bits<3> funct> {
+class POOL32C_STORE_EVA_FM_MMR6<bits<4> funct, bits<3> fmt> {
bits<5> rt;
bits<21> addr;
bits<5> base = addr{20-16};
bits<9> offset = addr{8-0};
bits<32> Inst;
let Inst{31-26} = 0b011000;
let Inst{25-21} = rt;
let Inst{20-16} = base;
- let Inst{15-12} = 0b1010;
- let Inst{11-9} = funct;
+ let Inst{15-12} = funct;
+ let Inst{11-9} = fmt;
let Inst{8-0} = offset;
}
-class LOAD_WORD_EVA_FM_MMR6<bits<3> funct> {
+class POOL32C_LOAD_WORD_EVA_FM_MMR6<bits<4> funct, bits<3> fmt> {
bits<5> rt;
bits<21> addr;
bits<5> base = addr{20-16};
bits<9> offset = addr{8-0};
bits<32> Inst;
let Inst{31-26} = 0b011000;
let Inst{25-21} = rt;
let Inst{20-16} = base;
- let Inst{15-12} = 0b0110;
- let Inst{11-9} = funct;
+ let Inst{15-12} = funct;
+ let Inst{11-9} = fmt;
let Inst{8-0} = offset;
----------------
Is there a need to provide bits 15-12 in an argument? We always pass the same value as far as I can see.
If it's really necessary: this is a different change to the one described in the description and should be in a separate patch.
================
Comment at: lib/Target/Mips/MicroMips32r6InstrInfo.td:912-938
@@ -911,25 +911,29 @@
class SB_MMR6_DESC : STORE_MMR6_DESC_BASE<"sb", GPR32Opnd>;
-class STORE_EVA_MMR6_DESC_BASE<string instr_asm, RegisterOperand RO>
- : MMR6Arch<instr_asm>, MipsR6Inst {
+class STORE_EVA_MMR6_DESC_BASE<string opstr, RegisterOperand RO,
+ Operand ImmOpnd>
+ : MMR6Arch<opstr>, MipsR6Inst {
dag OutOperandList = (outs);
- dag InOperandList = (ins RO:$rt, mem_mm_9:$addr);
- string AsmString = !strconcat(instr_asm, "\t$rt, $addr");
+ dag InOperandList = (ins RO:$rt, ImmOpnd:$addr);
+ string AsmString = !strconcat(opstr, "\t$rt, $addr");
string DecoderMethod = "DecodeStoreEvaOpMM";
bit mayStore = 1;
}
-class SBE_MMR6_DESC : STORE_EVA_MMR6_DESC_BASE<"sbe", GPR32Opnd>;
-class SCE_MMR6_DESC : STORE_EVA_MMR6_DESC_BASE<"sce", GPR32Opnd>;
+class SBE_MMR6_DESC : STORE_EVA_MMR6_DESC_BASE<"sbe", GPR32Opnd, mem_simm9gpr>;
+class SCE_MMR6_DESC : STORE_EVA_MMR6_DESC_BASE<"sce", GPR32Opnd, mem_simm9gpr>;
class SH_MMR6_DESC : STORE_MMR6_DESC_BASE<"sh", GPR32Opnd>;
-class SHE_MMR6_DESC : STORE_EVA_MMR6_DESC_BASE<"she", GPR32Opnd>;
-class LOAD_WORD_EVA_MMR6_DESC_BASE<string instr_asm, RegisterOperand RO> :
- MMR6Arch<instr_asm>, MipsR6Inst {
+class SHE_MMR6_DESC : STORE_EVA_MMR6_DESC_BASE<"she", GPR32Opnd, mem_simm9gpr>;
+class LOAD_WORD_EVA_MMR6_DESC_BASE<string opstr, RegisterOperand RO,
+ Operand ImmOpnd>
+ : MMR6Arch<opstr>, MipsR6Inst {
dag OutOperandList = (outs RO:$rt);
- dag InOperandList = (ins mem_mm_12:$addr);
- string AsmString = !strconcat(instr_asm, "\t$rt, $addr");
+ dag InOperandList = (ins ImmOpnd:$addr);
+ string AsmString = !strconcat(opstr, "\t$rt, $addr");
string DecoderMethod = "DecodeMemMMImm9";
bit mayLoad = 1;
}
-class LLE_MMR6_DESC : LOAD_WORD_EVA_MMR6_DESC_BASE<"lle", GPR32Opnd>;
-class LWE_MMR6_DESC : LOAD_WORD_EVA_MMR6_DESC_BASE<"lwe", GPR32Opnd>;
+class LLE_MMR6_DESC : LOAD_WORD_EVA_MMR6_DESC_BASE<"lle", GPR32Opnd,
+ mem_simm9gpr>;
+class LWE_MMR6_DESC : LOAD_WORD_EVA_MMR6_DESC_BASE<"lwe", GPR32Opnd,
+ mem_simm9gpr>;
class ADDU16_MMR6_DESC : ArithRMM16<"addu16", GPRMM16Opnd, 1, II_ADDU, add>,
----------------
Similarly, is there a need for ImmOpnd to be an argument? We only use one value as far as I can see.
The mem_mm_12 -> mem_simm9gpr change is correct. Thanks for spotting it since I missed it in the r265010 I committed earlier today.
================
Comment at: lib/Target/Mips/MipsEVAInstrInfo.td:54-84
@@ -53,31 +53,33 @@
// Memory Load/Store EVA descriptions
-class LOAD_EVA_DESC_BASE<string instr_asm, RegisterOperand GPROpnd> {
+class LOAD_EVA_DESC_BASE<string instr_asm, RegisterOperand GPROpnd,
+ Operand ImmOpnd> {
dag OutOperandList = (outs GPROpnd:$rt);
- dag InOperandList = (ins mem_simm9:$addr);
+ dag InOperandList = (ins ImmOpnd:$addr);
string AsmString = !strconcat(instr_asm, "\t$rt, $addr");
list<dag> Pattern = [];
string DecoderMethod = "DecodeMemEVA";
bit canFoldAsLoad = 1;
bit mayLoad = 1;
}
-class LBE_DESC : LOAD_EVA_DESC_BASE<"lbe", GPR32Opnd>;
-class LBuE_DESC : LOAD_EVA_DESC_BASE<"lbue", GPR32Opnd>;
-class LHE_DESC : LOAD_EVA_DESC_BASE<"lhe", GPR32Opnd>;
-class LHuE_DESC : LOAD_EVA_DESC_BASE<"lhue", GPR32Opnd>;
-class LWE_DESC : LOAD_EVA_DESC_BASE<"lwe", GPR32Opnd>;
+class LBE_DESC : LOAD_EVA_DESC_BASE<"lbe", GPR32Opnd, mem_simm9>;
+class LBuE_DESC : LOAD_EVA_DESC_BASE<"lbue", GPR32Opnd, mem_simm9>;
+class LHE_DESC : LOAD_EVA_DESC_BASE<"lhe", GPR32Opnd, mem_simm9>;
+class LHuE_DESC : LOAD_EVA_DESC_BASE<"lhue", GPR32Opnd, mem_simm9>;
+class LWE_DESC : LOAD_EVA_DESC_BASE<"lwe", GPR32Opnd, mem_simm9gpr>;
class STORE_EVA_DESC_BASE<string instr_asm, RegisterOperand GPROpnd,
+ Operand ImmOpnd,
SDPatternOperator OpNode = null_frag> {
dag OutOperandList = (outs);
- dag InOperandList = (ins GPROpnd:$rt, mem_simm9:$addr);
+ dag InOperandList = (ins GPROpnd:$rt, ImmOpnd:$addr);
string AsmString = !strconcat(instr_asm, "\t$rt, $addr");
list<dag> Pattern = [];
string DecoderMethod = "DecodeMemEVA";
bit mayStore = 1;
}
-class SBE_DESC : STORE_EVA_DESC_BASE<"sbe", GPR32Opnd>;
-class SHE_DESC : STORE_EVA_DESC_BASE<"she", GPR32Opnd>;
-class SWE_DESC : STORE_EVA_DESC_BASE<"swe", GPR32Opnd>;
+class SBE_DESC : STORE_EVA_DESC_BASE<"sbe", GPR32Opnd, mem_simm9gpr>;
+class SHE_DESC : STORE_EVA_DESC_BASE<"she", GPR32Opnd, mem_simm9gpr>;
+class SWE_DESC : STORE_EVA_DESC_BASE<"swe", GPR32Opnd, mem_simm9>;
----------------
mem_simm9 and mem_simm9gpr appear to be the same thing except for the predicate function, and it turns out that the body of those predicates are also equivalent. We should replace all uses of mem_simm9gpr with mem_simm9 and remove the code duplication.
http://reviews.llvm.org/D18645
More information about the llvm-commits
mailing list