[Subzero][MIPS32] Implement icmp operation for i8, i16 operands

src/IceTargetLoweringMIPS32.cpp

Index: src/IceTargetLoweringMIPS32.cpp
diff --git a/src/IceTargetLoweringMIPS32.cpp b/src/IceTargetLoweringMIPS32.cpp
index 66116e3c2f9a7f008623fdd55742a49015f6f097..1039ee89087d121c63e520a9b947b4e33f468c45 100644
--- a/src/IceTargetLoweringMIPS32.cpp
+++ b/src/IceTargetLoweringMIPS32.cpp
@@ -1157,11 +1157,50 @@ void TargetMIPS32::lowerIcmp(const InstIcmp *Instr) {
   InstIcmp::ICond Cond = Instr->getCondition();
   auto *Src0R = legalizeToReg(Src0);
   auto *Src1R = legalizeToReg(Src1);
+  const Type Src0Ty = Src0R->getType();
+  const uint32_t ShAmt = INT32_BITS - getScalarIntBitWidth(Src0->getType());
+  const uint32_t Mask = (1 << getScalarIntBitWidth(Src0->getType())) - 1;
+  Variable *Src0RT = I32Reg();
+  Variable *Src1RT = I32Reg();
+
+  if (Src0Ty != IceType_i32) {
+    Variable *T1 = I32Reg();
+    Variable *T2 = I32Reg();
+    switch (Cond) {
+    default:
+      llvm_unreachable("Invalid ICmp operator");
+      break;
+    case InstIcmp::Eq:
+    case InstIcmp::Ne:
+    case InstIcmp::Ugt:
+    case InstIcmp::Uge:
+    case InstIcmp::Ult:
+    case InstIcmp::Ule: {
+      _andi(Src0RT, Src0R, Mask);

[John, 2016/05/31 14:16:38]

For ARM, Subzero does not bother clearing the upper bits in non-32-bit
arithmetic, ensuring that writes to memory (and loads), and comparisons only
look at the "right" bits.

When comparing to N-bit integers (N is 1, 8, or 16), the ARM backend lsl the
operands (shifts left) and compares the resulting operands (no right shift.)

For example, if an operand is an i8, the lowering sequence would be:

mov T0, Src0
mov T1, Src1
lsl T0, T0, 24
cmp T0, T1 lsl 24

notice that by shifting the operand left by 24 bits, the operands most
significant bit (the sign bit) ends up in bit 32 of the destination register.

I think MIPS could do something similar (i.e., simply left shift the operands.)
What do you think? For the signed case this could save two instructions per
comparison.

[sagar.thakur, 2016/06/01 13:17:22]

Yes, I agree with this approach. Updated the patch to reflect the same. Thanks.

+      _andi(Src1RT, Src1R, Mask);
+      break;
+    }
+    case InstIcmp::Sgt:
+    case InstIcmp::Sge:
+    case InstIcmp::Slt:
+    case InstIcmp::Sle: {
+      _sll(T1, Src0R, ShAmt);
+      _sra(Src0RT, T1, ShAmt);
+      _sll(T2, Src1R, ShAmt);
+      _sra(Src1RT, T2, ShAmt);
+      break;
+    }
+    }
+  } else {
+    _mov(Src0RT, Src0R);
+    _mov(Src1RT, Src1R);
+  }
+
   switch (Cond) {
   case InstIcmp::Eq: {
     auto *DestT = I32Reg();
     auto *T = I32Reg();
-    _xor(T, Src0R, Src1R);
+    _xor(T, Src0RT, Src1RT);
     _sltiu(DestT, T, 1);
     _mov(Dest, DestT);
     return;
@@ -1170,63 +1209,63 @@ void TargetMIPS32::lowerIcmp(const InstIcmp *Instr) {
     auto *DestT = I32Reg();
     auto *T = I32Reg();
     auto *Zero = getZero();
-    _xor(T, Src0R, Src1R);
+    _xor(T, Src0RT, Src1RT);
     _sltu(DestT, Zero, T);
     _mov(Dest, DestT);
     return;
   }
   case InstIcmp::Ugt: {
     auto *DestT = I32Reg();
-    _sltu(DestT, Src1R, Src0R);
+    _sltu(DestT, Src1RT, Src0RT);
     _mov(Dest, DestT);
     return;
   }
   case InstIcmp::Uge: {
     auto *DestT = I32Reg();
     auto *T = I32Reg();
-    _sltu(T, Src0R, Src1R);
+    _sltu(T, Src0RT, Src1RT);
     _xori(DestT, T, 1);
     _mov(Dest, DestT);
     return;
   }
   case InstIcmp::Ult: {
     auto *DestT = I32Reg();
-    _sltu(DestT, Src0R, Src1R);
+    _sltu(DestT, Src0RT, Src1RT);
     _mov(Dest, DestT);
     return;
   }
   case InstIcmp::Ule: {
     auto *DestT = I32Reg();
     auto *T = I32Reg();
-    _sltu(T, Src1R, Src0R);
+    _sltu(T, Src1RT, Src0RT);
     _xori(DestT, T, 1);
     _mov(Dest, DestT);
     return;
   }
   case InstIcmp::Sgt: {
     auto *DestT = I32Reg();
-    _slt(DestT, Src1R, Src0R);
+    _slt(DestT, Src1RT, Src0RT);
     _mov(Dest, DestT);
     return;
   }
   case InstIcmp::Sge: {
     auto *DestT = I32Reg();
     auto *T = I32Reg();
-    _slt(T, Src0R, Src1R);
+    _slt(T, Src0RT, Src1RT);
     _xori(DestT, T, 1);
     _mov(Dest, DestT);
     return;
   }
   case InstIcmp::Slt: {
     auto *DestT = I32Reg();
-    _slt(DestT, Src0R, Src1R);
+    _slt(DestT, Src0RT, Src1RT);
     _mov(Dest, DestT);
     return;
   }
   case InstIcmp::Sle: {
     auto *DestT = I32Reg();
     auto *T = I32Reg();
-    _slt(T, Src1R, Src0R);
+    _slt(T, Src1RT, Src0RT);
     _xori(DestT, T, 1);
     _mov(Dest, DestT);
     return;