[PATCH] D19354: [mips][microMIPS] Implement SLT, SLTI, SLTIU, SLTU microMIPS32r6 instructions

[Hrvoje Varga via llvm-commits]

hvarga added inline comments.

================
Comment at: lib/Target/Mips/MicroMips32r6InstrInfo.td:984-986
@@ -983,3 +983,5 @@
 class ADDU16_MMR6_DESC : ArithRMM16<"addu16", GPRMM16Opnd, 1, II_ADDU, add>,
-      MMR6Arch<"addu16">;
+      MMR6Arch<"addu16"> {
+  int AddedComplexity  = 1;
+}
 class AND16_MMR6_DESC : LogicRMM16<"and16", GPRMM16Opnd, II_AND, and>,
----------------
sdardis wrote:
> This change should be separate. Also, what's the reason behind this change?
Well actually, It can't be in a separate patch. :)

The reason is that I needed to remove setting `list<dag> Pattern = [];` in `MMArch` class. Removing this caused test `test/CodeGen/Mips/llvm-ir/add.ll` to fail because of wrong instruction selection. In this test, it was expected to select the instruction ADDU16 but instead, instruction ADDU is selected. So, `ADDu_MM` is selected instead of `ADDU16_MMR6`.

Using the `llvm-tblgen` tool I have discovered that `ADDu_MM` is selected because this definition now, after removing the Pattern in `MMArch`, has a correct `Pattern`.

So the `MMArch` class is actually removing the `Pattern` which was, for `ADDu_MM`, defined in a class `ArithLogicR`. From my point, this is not the desired behavior.

It can be seen from the `llvm-tblgen` output that for the `ADDu_MM` the `MMArch` is set after the `ArithLogicR` in the inheritance chain. Hence, `MMArch` overrides the `Pattern` which was set in `ArithLogicR` class.

I am certain that this problem is also manifesting for `SLT_MM` and probably couple more instructions from this patch and the rest of codebase.



================
Comment at: lib/Target/Mips/MipsInstrInfo.td:1651-1661
@@ -1650,11 +1650,13 @@
             ISA_MIPS1_NOT_32R6_64R6;
+let AdditionalPredicates = [NotInMicroMips] in {
 def SLTi  : MMRel, SetCC_I<"slti", setlt, simm16, immSExt16, GPR32Opnd>,
             SLTI_FM<0xa>;
 def SLTiu : MMRel, SetCC_I<"sltiu", setult, simm16, immSExt16, GPR32Opnd>,
             SLTI_FM<0xb>;
+}
 let AdditionalPredicates = [NotInMicroMips] in {
 def ANDi  : MMRel, StdMMR6Rel,
             ArithLogicI<"andi", uimm16, GPR32Opnd, II_ANDI, immZExt16, and>,
             ADDI_FM<0xc>;
 }
 def ORi   : MMRel, StdMMR6Rel,
----------------
sdardis wrote:
> Join these blocks together.
Will do.


http://reviews.llvm.org/D19354