[PATCH] D10923: [mips][microMIPS] Create microMIPS64r6 subtarget and implement DALIGN, DAUI, DAHI, DATI, DEXT, DEXTM and DEXTU instructions

Daniel Sanders daniel.sanders at imgtec.com
Mon Jul 6 07:03:24 PDT 2015


================
Comment at: lib/Target/Mips/Disassembler/MipsDisassembler.cpp:850
@@ -846,3 +849,3 @@
 
-    if (hasMips32r6()) {
+    if (hasMips32r6() || hasMips64r6()) {
       DEBUG(dbgs() << "Trying MicroMips32r632 table (32-bit instructions):\n");
----------------
hasMips64r6() isn't needed. The predicates are cumulative so hasMips32r6() is true for MIPS64r6 too.

================
Comment at: lib/Target/Mips/MicroMips32r6InstrInfo.td:286
@@ -285,3 +285,3 @@
 
-let DecoderNamespace = "MicroMips32r6" in {
+let DecoderNamespace = "MicroMips32r6_64r6" in {
 def ADD_MMR6 : StdMMR6Rel, ADD_MMR6_DESC, ADD_MMR6_ENC, ISA_MICROMIPS32R6;
----------------
I'm on the fence about this. For consistency with the other namespaces, we shouldn't add '_64r6' but equally more descriptive names are generally a good thing.

================
Comment at: lib/Target/Mips/MicroMips64r6InstrFormats.td:1
@@ +1,2 @@
+//=- MicroMips64r6InstrFormats.td - Mips64r6 Instruction Formats -*- tablegen -*-==//
+//
----------------
80 cols?

================
Comment at: lib/Target/Mips/MicroMips64r6InstrFormats.td:14
@@ +13,3 @@
+
+class DOUBLE_ADD_UPPER_IMM {
+  bits<5> rt;
----------------
Please use the naming convention we used for MIPS32r6/MIPS64r6.

The prefix should match the major opcode name. Then (where necessary) something to disambiguate variations of the major opcode. Lastly a '_FM' suffix.

For example:
    DOUBLE_ADD_UPPER_IMM -> DAUI_FM.
    DOUBLE_EXT_BIT_FIELD -> POOL32S_EXTBITS_FM.
    CONCAT_EXTRACT -> POOL32S_DALIGN_FM.

================
Comment at: lib/Target/Mips/MicroMips64r6InstrInfo.td:1
@@ +1,2 @@
+//=- MicroMips64r6InstrInfo.td - MicroMips64r6 Instruction Information -*- tablegen -*-=//
+//
----------------
80 cols?

================
Comment at: lib/Target/Mips/MicroMips64r6InstrInfo.td:43
@@ +42,3 @@
+
+class DOUBLE_ADD_HIGHER_TOP_MMR6_DESC_BASE<string instr_asm,
+                                           RegisterOperand GPROpnd> :
----------------
dsanders wrote:
> Indentation
Optional nit: DAHI_DATI_DESC_BASE? It's easier to format when the names aren't so long and we generally don't expand the mnemonic to the words it stands for.

Similarly for the other lengthy *_DESC_BASE's below.

================
Comment at: lib/Target/Mips/MicroMips64r6InstrInfo.td:43-45
@@ +42,5 @@
+
+class DOUBLE_ADD_HIGHER_TOP_MMR6_DESC_BASE<string instr_asm,
+                                           RegisterOperand GPROpnd> :
+			                                         MMR6Arch<instr_asm>, MipsR6Inst {
+  dag OutOperandList = (outs GPROpnd:$rs);
----------------
Indentation

================
Comment at: lib/Target/Mips/MicroMips64r6InstrInfo.td:56-57
@@ +55,4 @@
+                                Operand PosOpnd,
+                                SDPatternOperator Op = null_frag> :
+                                  MMR6Arch<instr_asm>, MipsR6Inst {
+  dag OutOperandList = (outs RO:$rt);
----------------
Indendation

================
Comment at: lib/Target/Mips/MicroMips64r6InstrInfo.td:90-100
@@ +89,13 @@
+let DecoderNamespace = "MicroMips32r6_64r6" in {
+  def DAUI_MM64R6 : R6MMR6Rel, DAUI_MMR6_DESC, DAUI_MMR6_ENC, ISA_MICROMIPS64R6;
+  def DAHI_MM64R6 : R6MMR6Rel, DAHI_MMR6_DESC, DAHI_MMR6_ENC, ISA_MICROMIPS64R6;
+  def DATI_MM64R6 : R6MMR6Rel, DATI_MMR6_DESC, DATI_MMR6_ENC, ISA_MICROMIPS64R6;
+  def DEXT_MM64R6 : StdMMR6Rel, DEXT_MMR6_DESC, DEXT_MMR6_ENC,
+                    ISA_MICROMIPS64R6;
+  def DEXTM_MM64R6 : StdMMR6Rel, DEXTM_MMR6_DESC, DEXTM_MMR6_ENC,
+                     ISA_MICROMIPS64R6;
+  def DEXTU_MM64R6 : StdMMR6Rel, DEXTU_MMR6_DESC, DEXTU_MMR6_ENC,
+                     ISA_MICROMIPS64R6;
+  def DALIGN_MM64R6 : R6MMR6Rel, DALIGN_MMR6_DESC, DALIGN_MMR6_ENC,
+                      ISA_MICROMIPS64R6;
+}
----------------
Why do some of these use StdMMR6Rel and others use R6MM6Rel?


http://reviews.llvm.org/D10923







More information about the llvm-commits mailing list