[PATCH] D16676: [mips][microMIPS] Add CodeGen support for SUBU16, SUB, SUBU, DSUB and DSUBU instructions

[Simon Dardis via llvm-commits]

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

Comments inlined.

Can you add invalid tests for dneg(u) with an immediate operand?

In MipsSEDAGToDAGISel::selectNode(SDNode *Node), there is an expansion for the ISD:SUBE node which currently expands to Mips::DSUBu / Mips::SUBu. You will need to extend that to cover micromips.


================
Comment at: lib/Target/Mips/MicroMips64r6InstrFormats.td:87-102
@@ -86,1 +86,17 @@
 }
+
+class POOL32S_DSUB_FM_MMR6<string instr_asm, bits<9> funct>
+    : MMR6Arch<instr_asm> {
+  bits<5> rd;
+  bits<5> rs;
+  bits<5> rt;
+
+  bits<32> Inst;
+
+  let Inst{31-26} = 0b010110;
+  let Inst{25-21} = rt;
+  let Inst{20-16} = rs;
+  let Inst{15-11} = rd;
+  let Inst{10-9}  = 0;
+  let Inst{8-0}   = funct;
+}
----------------
Rather than adding this specific class for dsub(u), can you use POOL32SARITH_FM_MMR6 instead of POOL32S_DSUB? They share the same major opcode.

================
Comment at: lib/Target/Mips/MicroMips64r6InstrInfo.td:31-32
@@ -30,2 +30,4 @@
 class DMODU_MM64R6_ENC : POOL32A_DIVMOD_FM_MMR6<"dmodu", 0b111011000>;
+class DSUB_MM64R6_ENC : POOL32S_DSUB_FM_MMR6<"dsub", 0b110010000>;
+class DSUBU_MM64R6_ENC : POOL32S_DSUB_FM_MMR6<"dsubu", 0b111010000>;
 
----------------
See my previous comment.

================
Comment at: lib/Target/Mips/MicroMips64r6InstrInfo.td:169
@@ +168,2 @@
+                    (DSUBU_MM64R6 GPR64Opnd:$rt, ZERO_64, GPR64Opnd:$rs), 1>,
+                    ISA_MICROMIPS64R6;
----------------
One more pattern required, "dnegu $rt".

================
Comment at: lib/Target/Mips/Mips64InstrInfo.td:124-129
@@ -123,6 +123,8 @@
              ISA_MIPS3;
-def DSUBu  : ArithLogicR<"dsubu", GPR64Opnd, 0, II_DSUBU, sub>, ADD_FM<0, 0x2f>,
-             ISA_MIPS3;
-def DSUB   : ArithLogicR<"dsub", GPR64Opnd, 0, II_DSUB>, ADD_FM<0, 0x2e>,
-             ISA_MIPS3;
+let AdditionalPredicates = [NotInMicroMips] in {
+  def DSUBu  : StdMMR6Rel, ArithLogicR<"dsubu", GPR64Opnd, 0, II_DSUBU, sub>, ADD_FM<0, 0x2f>,
+               ISA_MIPS3;
+  def DSUB   : StdMMR6Rel, ArithLogicR<"dsub", GPR64Opnd, 0, II_DSUB>, ADD_FM<0, 0x2e>,
+               ISA_MIPS3;
+}
 
----------------
After you rebase, you can include DSUBu and DSUB in the same block ass DADD and DADDU.


http://reviews.llvm.org/D16676