[PATCH] D12141: [mips][microMIPS] Implement CVT.D.fmt, CVT.L.fmt, CVT.S.fmt, CVT.W.fmt, MAX.fmt, MIN.fmt, MAXA.fmt, MINA.fmt and CMP.condn.fmt instructions

[Daniel Sanders via llvm-commits]

dsanders accepted this revision.
dsanders added a comment.
This revision is now accepted and ready to land.

LGTM with some indentation nits.

There's currently no clang-format for tablegen files but for consistency with the existing definitions, try to format tablegen definitions the same way clang-format would do C/C++. For example, the inheritance lists (the right hand side of the ':') are formatted roughly the same way as initialization lists on a C++ constructor, class arguments are formatted the same way as arguments on a C/C++ function, curly braces increase indentation, etc.


================
Comment at: lib/Target/Mips/MicroMips32r6InstrInfo.td:346-348
@@ +345,5 @@
+
+class CVT_MMR6_DESC_BASE<string instr_asm, RegisterOperand DstRC,
+    RegisterOperand SrcRC, InstrItinClass Itin,
+    SDPatternOperator OpNode = null_frag> : HARDFLOAT, NeverHasSideEffects {
+  dag OutOperandList = (outs DstRC:$ft);
----------------
Indentation nit: A hanging indent like this makes sense given the amount of wrapping that's needed but either all arguments should be hanging or none. In this case the first arguments should be hanging too.

  class CVT_MMR6_DESC_BASE<
      string instr_asm, RegisterOperand DstRC,
      RegisterOperand SrcRC, InstrItinClass Itin,
      ...

================
Comment at: lib/Target/Mips/MicroMips32r6InstrInfo.td:358
@@ +357,3 @@
+class CVT_L_S_MMR6_DESC : CVT_MMR6_DESC_BASE<"cvt.l.s", FGR64Opnd, FGR32Opnd,
+    II_CVT>;
+class CVT_L_D_MMR6_DESC : CVT_MMR6_DESC_BASE<"cvt.l.d", FGR64Opnd, FGR64Opnd,
----------------
Indentation nit: Should be aligned to the first argument.

Likewise for the others

================
Comment at: lib/Target/Mips/MicroMips32r6InstrInfo.td:380-382
@@ +379,5 @@
+    RegisterOperand FGROpnd> {
+  def CMP_AF_#NAME : POOL32F_CMP_FM<!strconcat("cmp.af.", Typestr), format,
+      FIELD_CMP_COND_AF>, CMP_CONDN_DESC_BASE<"af", Typestr, FGROpnd>,
+      HARDFLOAT, R6MMR6Rel, ISA_MICROMIPS32R6;
+  def CMP_UN_#NAME : POOL32F_CMP_FM<!strconcat("cmp.un.", Typestr), format,
----------------
Indentation nit: As mentioned above, hanging indents are sensible here but it should be all arguments or none.

================
Comment at: lib/Target/Mips/Mips32r6InstrInfo.td:710-717
@@ -707,8 +709,10 @@
+let AdditionalPredicates = [NotInMicroMips] in {
 def MAXA_D  : MAXA_D_ENC, MAXA_D_DESC, ISA_MIPS32R6, HARDFLOAT;
 def MAXA_S  : MAXA_S_ENC, MAXA_S_DESC, ISA_MIPS32R6, HARDFLOAT;
 def MAX_D   : MAX_D_ENC, MAX_D_DESC, ISA_MIPS32R6, HARDFLOAT;
 def MAX_S   : MAX_S_ENC, MAX_S_DESC, ISA_MIPS32R6, HARDFLOAT;
 def MINA_D  : MINA_D_ENC, MINA_D_DESC, ISA_MIPS32R6, HARDFLOAT;
 def MINA_S  : MINA_S_ENC, MINA_S_DESC, ISA_MIPS32R6, HARDFLOAT;
 def MIN_D   : MIN_D_ENC, MIN_D_DESC, ISA_MIPS32R6, HARDFLOAT;
 def MIN_S   : MIN_S_ENC, MIN_S_DESC, ISA_MIPS32R6, HARDFLOAT;
+}
----------------
Indentation Nit: Should be indented because of the let statement

================
Comment at: lib/Target/Mips/MipsInstrFPU.td:309-312
@@ -308,4 +308,6 @@
+let AdditionalPredicates = [NotInMicroMips] in{
 def CVT_L_S : MMRel, ABSS_FT<"cvt.l.s", FGR64Opnd, FGR32Opnd, II_CVT>,
               ABSS_FM<0x25, 16>, INSN_MIPS3_32R2;
 def CVT_L_D64: MMRel, ABSS_FT<"cvt.l.d", FGR64Opnd, FGR64Opnd, II_CVT>,
                ABSS_FM<0x25, 17>, INSN_MIPS3_32R2;
+}
----------------
Indentation Nit: Should be indented because of the let statement

================
Comment at: lib/Target/Mips/MipsInstrFPU.td:326-327
@@ -323,2 +325,4 @@
+  let AdditionalPredicates = [NotInMicroMips] in{
   def CVT_S_L   : ABSS_FT<"cvt.s.l", FGR32Opnd, FGR64Opnd, II_CVT>,
                   ABSS_FM<0x20, 21>, FGR_64;
+  }
----------------
Indentation Nit: Should be indented because of the let statement


http://reviews.llvm.org/D12141