[PATCH] D13885: [mips][microMIPS] Implement RECIP.fmt, RINT.fmt, ROUND.L.fmt, ROUND.W.fmt, SEL.fmt, SELEQZ.fmt, SELNEQZ.fmt and CLASS.fmt

Daniel Sanders via llvm-commits llvm-commits at lists.llvm.org
Thu Oct 22 07:33:39 PDT 2015


dsanders requested changes to this revision.
dsanders added a comment.
This revision now requires changes to proceed.

If you're using the web interface to create the review, please remember to include the full context as described at http://llvm.org/docs/Phabricator.html#requesting-a-review-via-the-web-interface

My main concern is the removal of several FPU instructions from 32R6 and 64R6.


================
Comment at: lib/Target/Mips/MicroMips32r6InstrInfo.td:115
@@ +114,3 @@
+class RECIP_S_MMR6_ENC : POOL32F_RECIP_ROUND_FM_MMR6<"recip.s", 0, 0b01001000>;
+class RECIP_MMR6_ENC : POOL32F_RECIP_ROUND_FM_MMR6<"recip.d", 1, 0b01001000>;
+class RINT_S_MMR6_ENC : POOL32F_RINT_FM_MMR6<"rint.s", 0>;
----------------
RECIP -> RECIP_D

================
Comment at: lib/Target/Mips/MicroMips32r6InstrInfo.td:117
@@ +116,3 @@
+class RINT_S_MMR6_ENC : POOL32F_RINT_FM_MMR6<"rint.s", 0>;
+class RINT_MMR6_ENC : POOL32F_RINT_FM_MMR6<"rint.d", 1>;
+class ROUND_L_S_MMR6_ENC : POOL32F_RECIP_ROUND_FM_MMR6<"round.l.s", 0,
----------------
RINT -> RINT_D

================
Comment at: lib/Target/Mips/MicroMips32r6InstrInfo.td:120
@@ +119,3 @@
+                                                       0b11001100>;
+class ROUND_L_MMR6_ENC : POOL32F_RECIP_ROUND_FM_MMR6<"round.l.d", 1,
+                                                     0b11001100>;
----------------
ROUND_L -> ROUND_L_D

================
Comment at: lib/Target/Mips/MicroMips32r6InstrInfo.td:124
@@ +123,3 @@
+                                                       0b11101100>;
+class ROUND_W_MMR6_ENC : POOL32F_RECIP_ROUND_FM_MMR6<"round.w.d", 1,
+                                                      0b11101100>;
----------------
ROUND_W -> ROUND_W_D

================
Comment at: lib/Target/Mips/MicroMips32r6InstrInfo.td:127
@@ +126,3 @@
+class SEL_S_MMR6_ENC : POOL32F_SEL_FM_MMR6<"sel.s", 0, 0b010111000>;
+class SEL_MMR6_ENC : POOL32F_SEL_FM_MMR6<"sel.d", 1, 0b010111000>;
+class SELEQZ_S_MMR6_ENC : POOL32F_SEL_FM_MMR6<"seleqz.s", 0, 0b000111000>;
----------------
SEL -> SEL_D

================
Comment at: lib/Target/Mips/MicroMips32r6InstrInfo.td:714-725
@@ -693,1 +713,14 @@
                                                   AFGR64Opnd, II_TRUNC>;
+class RECIP_S_MMR6_DESC : ABSS_FT_MMR6_DESC_BASE<"recip.s", FGR32Opnd,
+                                                 FGR32Opnd, II_ROUND>;
+class RECIP_MMR6_DESC : ABSS_FT_MMR6_DESC_BASE<"recip.d", FGR32Opnd, FGR32Opnd,
+                                               II_ROUND>;
+class ROUND_L_S_MMR6_DESC : ABSS_FT_MMR6_DESC_BASE<"round.l.s", FGR64Opnd,
+                                                   FGR32Opnd, II_ROUND>;
+class ROUND_L_MMR6_DESC : ABSS_FT_MMR6_DESC_BASE<"round.l.d", FGR64Opnd,
+                                                 FGR64Opnd, II_ROUND>;
+class ROUND_W_S_MMR6_DESC : ABSS_FT_MMR6_DESC_BASE<"round.w.s", FGR32Opnd,
+                                                   FGR32Opnd, II_ROUND>;
+class ROUND_W_MMR6_DESC : ABSS_FT_MMR6_DESC_BASE<"round.w.d", FGR64Opnd,
+                                                 FGR64Opnd, II_ROUND>;
+
----------------
I should have mentioned this sooner but ABSS_FT_MMR6_DESC_BASE seems to have become a generic 2-operand description. This being the case, could you rename it (in a follow-up patch)? It seems strange to see RECIP.S (etc.) described as being an absolute operation.

================
Comment at: lib/Target/Mips/MicroMips32r6InstrInfo.td:716
@@ +715,3 @@
+                                                 FGR32Opnd, II_ROUND>;
+class RECIP_MMR6_DESC : ABSS_FT_MMR6_DESC_BASE<"recip.d", FGR32Opnd, FGR32Opnd,
+                                               II_ROUND>;
----------------
RECIP -> RECIP_D

================
Comment at: lib/Target/Mips/MicroMips32r6InstrInfo.td:720
@@ +719,3 @@
+                                                   FGR32Opnd, II_ROUND>;
+class ROUND_L_MMR6_DESC : ABSS_FT_MMR6_DESC_BASE<"round.l.d", FGR64Opnd,
+                                                 FGR64Opnd, II_ROUND>;
----------------
ROUND_L -> ROUND_L_D

================
Comment at: lib/Target/Mips/MicroMips32r6InstrInfo.td:724
@@ +723,3 @@
+                                                   FGR32Opnd, II_ROUND>;
+class ROUND_W_MMR6_DESC : ABSS_FT_MMR6_DESC_BASE<"round.w.d", FGR64Opnd,
+                                                 FGR64Opnd, II_ROUND>;
----------------
ROUND_W -> ROUND_W_D

================
Comment at: lib/Target/Mips/MicroMips32r6InstrInfo.td:728
@@ +727,3 @@
+class SEL_S_MMR6_DESC : COP1_SEL_DESC_BASE<"sel.s", FGR32Opnd>;
+class SEL_MMR6_DESC : COP1_SEL_DESC_BASE<"sel.d", FGR64Opnd> {
+  // We must insert a SUBREG_TO_REG around $fd_in
----------------
SEL -> SEL_D

================
Comment at: lib/Target/Mips/MicroMips32r6InstrInfo.td:738
@@ +737,3 @@
+class RINT_S_MMR6_DESC : CLASS_RINT_DESC_BASE<"rint.s", FGR32Opnd>;
+class RINT_MMR6_DESC : CLASS_RINT_DESC_BASE<"rint.d", FGR64Opnd>;
+class CLASS_S_MMR6_DESC  : CLASS_RINT_DESC_BASE<"class.s", FGR32Opnd>;
----------------
RINT -> RINT_D

================
Comment at: lib/Target/Mips/MicroMips32r6InstrInfo.td:1053
@@ +1052,3 @@
+                   ISA_MICROMIPS32R6;
+def RECIP_MMR6 : StdMMR6Rel, RECIP_MMR6_ENC, RECIP_MMR6_DESC, ISA_MICROMIPS32R6;
+def RINT_S_MMR6 : StdMMR6Rel, RINT_S_MMR6_ENC, RINT_S_MMR6_DESC,
----------------
RECIP -> RECIP_D

================
Comment at: lib/Target/Mips/MicroMips32r6InstrInfo.td:1056
@@ +1055,3 @@
+                  ISA_MICROMIPS32R6;
+def RINT_MMR6 : StdMMR6Rel, RINT_MMR6_ENC, RINT_MMR6_DESC, ISA_MICROMIPS32R6;
+def ROUND_L_S_MMR6 : StdMMR6Rel, ROUND_L_S_MMR6_ENC, ROUND_L_S_MMR6_DESC,
----------------
RINT -> RINT_D

================
Comment at: lib/Target/Mips/MicroMips32r6InstrInfo.td:1059
@@ +1058,3 @@
+                     ISA_MICROMIPS32R6;
+def ROUND_L_MMR6 : StdMMR6Rel, ROUND_L_MMR6_ENC, ROUND_L_MMR6_DESC,
+                   ISA_MICROMIPS32R6;
----------------
ROUND_L -> ROUND_L_D

================
Comment at: lib/Target/Mips/MicroMips32r6InstrInfo.td:1063
@@ +1062,3 @@
+                     ISA_MICROMIPS32R6;
+def ROUND_W_MMR6 : StdMMR6Rel, ROUND_W_MMR6_ENC, ROUND_W_MMR6_DESC,
+                   ISA_MICROMIPS32R6;
----------------
ROUND_W -> ROUND_W_D

================
Comment at: lib/Target/Mips/MicroMips32r6InstrInfo.td:1066
@@ +1065,3 @@
+def SEL_S_MMR6 : StdMMR6Rel, SEL_S_MMR6_ENC, SEL_S_MMR6_DESC, ISA_MICROMIPS32R6;
+def SEL_MMR6 : StdMMR6Rel, SEL_MMR6_ENC, SEL_MMR6_DESC, ISA_MICROMIPS32R6;
+def SELEQZ_S_MMR6 : StdMMR6Rel, SELEQZ_S_MMR6_ENC, SELEQZ_S_MMR6_DESC,
----------------
SEL -> SEL_D

================
Comment at: lib/Target/Mips/Mips32r6InstrInfo.td:716-725
@@ -718,1 +715,12 @@
+  def MIN_S : MIN_S_ENC, MIN_S_DESC, ISA_MIPS32R6, HARDFLOAT;
+  def RINT_D : RINT_D_ENC, RINT_D_DESC, ISA_MIPS32R6, HARDFLOAT;
+  def RINT_S : RINT_S_ENC, RINT_S_DESC, ISA_MIPS32R6, HARDFLOAT;
+  def SEL_D : SEL_D_ENC, SEL_D_DESC, ISA_MIPS32R6, HARDFLOAT;
+  def SEL_S : SEL_S_ENC, SEL_S_DESC, ISA_MIPS32R6, HARDFLOAT;
+  def CLASS_D : CLASS_D_ENC, CLASS_D_DESC, ISA_MIPS32R6, HARDFLOAT;
+  def CLASS_S : CLASS_S_ENC, CLASS_S_DESC, ISA_MIPS32R6, HARDFLOAT;
+  def SELNEZ_D : SELNEZ_D_ENC, SELNEZ_D_DESC, ISA_MIPS32R6, HARDFLOAT;
+  def SELNEZ_S : SELNEZ_S_ENC, SELNEZ_S_DESC, ISA_MIPS32R6, HARDFLOAT;
+  def SELEQZ_D : SELEQZ_D_ENC, SELEQZ_D_DESC, ISA_MIPS32R6, HARDFLOAT;
+  def SELEQZ_S : SELEQZ_S_ENC, SELEQZ_S_DESC, ISA_MIPS32R6, HARDFLOAT;
 }
----------------
Please move these back to where they were and use a separate 'let AdditionalPredicates ...'. They were in documentation order.

================
Comment at: lib/Target/Mips/MipsInstrFPU.td:270-279
@@ -269,12 +269,12 @@
 //===----------------------------------------------------------------------===//
 def ROUND_W_S  : MMRel, ABSS_FT<"round.w.s", FGR32Opnd, FGR32Opnd, II_ROUND>,
-                 ABSS_FM<0xc, 16>, ISA_MIPS2;
-let AdditionalPredicates = [NotInMicroMips] in {
+                 ABSS_FM<0xc, 16>, ISA_MIPS2, ISA_MIPS2_NOT_32R6_64R6;
+defm ROUND_W : ROUND_M<"round.w.d", II_ROUND>, ABSS_FM<0xc, 17>, ISA_MIPS2,
+               ISA_MIPS2_NOT_32R6_64R6;
 def TRUNC_W_S  : MMRel, StdMMR6Rel, ABSS_FT<"trunc.w.s", FGR32Opnd, FGR32Opnd, II_TRUNC>,
-                 ABSS_FM<0xd, 16>, ISA_MIPS2;
+                 ABSS_FM<0xd, 16>, ISA_MIPS2, ISA_MIPS2_NOT_32R6_64R6;
 def CEIL_W_S   : MMRel, StdMMR6Rel, ABSS_FT<"ceil.w.s", FGR32Opnd, FGR32Opnd, II_CEIL>,
-                 ABSS_FM<0xe, 16>, ISA_MIPS2;
+                 ABSS_FM<0xe, 16>, ISA_MIPS2, ISA_MIPS2_NOT_32R6_64R6;
 def FLOOR_W_S  : MMRel, StdMMR6Rel, ABSS_FT<"floor.w.s", FGR32Opnd, FGR32Opnd, II_FLOOR>,
-                 ABSS_FM<0xf, 16>, ISA_MIPS2;
-}
+                 ABSS_FM<0xf, 16>, ISA_MIPS2, ISA_MIPS2_NOT_32R6_64R6;
 def CVT_W_S    : MMRel, ABSS_FT<"cvt.w.s", FGR32Opnd, FGR32Opnd, II_CVT>,
----------------
Why have you removed these from 32R6/64R6? 32R6/64R6 didn't change these instructions

Also, multiple ISA_*'s on the same instruction doesn't work since it overrides the same sub-list from PredicateControl.


http://reviews.llvm.org/D13885





More information about the llvm-commits mailing list