[PATCH] implement integer compare in mips fast-isel

[Daniel Sanders]

================
Comment at: lib/Target/Mips/MipsFastISel.cpp:531
@@ -529,1 +530,3 @@
 
+bool MipsFastISel::SelectCmp(const Instruction *I) {
+  const CmpInst *CI = cast<CmpInst>(I);
----------------
dsanders wrote:
> Style nit: This function needs some vertical spacing.
> What kind of vertical spacing? I'm using clang-format on this file.

I mean it needs some blank lines to break up the logical sections of the code. For example, a blank line between the LHS/RHS handling, and just before the switch would help readability.

================
Comment at: lib/Target/Mips/MipsFastISel.cpp:537-553
@@ +536,19 @@
+    return false;
+  MVT LMVT = TLI.getValueType(Left->getType(), true).getSimpleVT();
+  if ((LMVT == MVT::i8) || (LMVT == MVT::i16)) {
+    unsigned TempReg = createResultReg(&Mips::GPR32RegClass);
+    if (!EmitIntExt(LMVT, LeftReg, MVT::i32, TempReg, CI->isUnsigned()))
+      return false;
+    LeftReg = TempReg;
+  }
+  unsigned RightReg = getRegForValue(Right);
+  if (RightReg == 0)
+    return false;
+  MVT RMVT = TLI.getValueType(Right->getType(), true).getSimpleVT();
+  if ((RMVT == MVT::i8) || (RMVT == MVT::i16)) {
+    unsigned TempReg = createResultReg(&Mips::GPR32RegClass);
+    if (!EmitIntExt(LMVT, RightReg, MVT::i32, TempReg, CI->isUnsigned()))
+      return false;
+    RightReg = TempReg;
+  }
+  unsigned ResultReg = createResultReg(&Mips::GPR32RegClass);
----------------
dsanders wrote:
> We've got some code duplication here. Both the LHS and the RHS are sign/zero extended in the same way, and we also duplicate the type checking that's inside EmitIntExt.
> 
> Can we factor it out?
> What do you mean about duplicating tests inside of EmitIntExt?
> Not sure what you are referring to.

At the moment, the caller is testing that LMVT is i8 or i16 and passes LMVT to EmitIntExt as the first argument. EmitIntExt also checks that first argument type is i8 or i16 (in its switch statement). It seems unnecessary to do it twice and have to maintain the list in both places. One possible solution is to call EmitIntExt unconditionally and have it simply return the input register when an extension isn't necessary (e.g. %1 = sext i32 %0 to i32).

http://reviews.llvm.org/D5566