[mips][FastISel] Implement the select statement for MIPS FastISel.

[vkalintiris]

Summary

Implement the LLVM IR select statement for MIPS FastISel.

Based on a patch by Reed Kotler.

[rkotler]

Simple executable test case.

sel1.c388 BDownload

[rkotler]

A regression was fixed here because it shows up in test-suite only after this patch. It's a generic problem caused by a shortcoming in tabelgen whereby you can't mark a side effect of an instruction as it making a register Dead and instead out tablegen marked the mul instruction wirh defs for LO0 and HI0 which causes a problem for the register allocator when trying to allocate register AC0 since LO0 and HI0 are dependent registers.

The solution here for this regression is probably temporary since the proper way is to fix tablegen but that will require some more work and approval.

[dsanders]

lib/Target/Mips/MipsFastISel.cpp

928–934 ↗

(On Diff #18202)

We don't need to check each integer type explictly. The generated code is correct for all integers of GPR size or smaller.

947 ↗

(On Diff #18202)

Nit: Useless statement

960 ↗

(On Diff #18202)

I think this is only needed if Src2Reg != DestReg. Is it possible for this code to emit something like:

move $2, $2
movn $2, $3, $4

964 ↗

(On Diff #18202)

TempReg isn't implicitly read. It's explicit in our instruction definition:

(outs DRC:$rd), (ins DRC:$rs, CRC:$rt, DRC:$F)

1660–1679 ↗

(On Diff #18202)

Why is this chunk in this patch? It looks we have two patches in one review.

test/CodeGen/Mips/Fast-ISel/sel1.ll

22 ↗

(On Diff #18202)

At this point we should have support for arguments and returns. Could you simplify the test using those?

I don't mind if we clean this up later but I'd like to have as little noise as possible in the tests.

[vkalintiris]

Addressed review comments and removed the review's portions that are not
strictly related to the implementation of the select instruction.

[dsanders]

LGTM with the type check corrected and the FIXMEs added.

lib/Target/Mips/MipsFastISel.cpp

906 ↗

(On Diff #25768)

MVT::INVALID_SIMPLE_VALUE_TYPE and MVT::Other are included in this check and shouldn't be.

Also, you can't rely on the types being defined in this order. You could use:

VT.isInteger() && !VT.isVector() && VT.getSizeInBits() <= 32

test/CodeGen/Mips/Fast-ISel/sel1.ll

8 ↗

(On Diff #25768)

Nit: Redundant instruction. You can leave a FIXME for now and follow up on it.

23–24 ↗

(On Diff #25768)

Nit: Redundant instructions. You can leave a FIXME for now and follow up on it.

39–40 ↗

(On Diff #25768)

Nit: Redundant instructions. You can leave a FIXME for now and follow up on it.

53 ↗

(On Diff #25768)

Nit: Redundant instruction. You can leave a FIXME for now and follow up on it.

68 ↗

(On Diff #25768)

Nit: Redundant instruction. You can leave a FIXME for now and follow up on it.

83 ↗

(On Diff #25768)

Nit: Redundant instruction. You can leave a FIXME for now and follow up on it.

llvm/trunk/lib/Target/Mips/MipsFastISel.cpp

class MipsFastISel final : public FastISel {
   // situations
 
 private:
   // Selection routines.
   bool selectLogicalOp(const Instruction *I);
   bool selectLoad(const Instruction *I);
   bool selectStore(const Instruction *I);
   bool selectBranch(const Instruction *I);
+  bool selectSelect(const Instruction *I);
   bool selectCmp(const Instruction *I);
   bool selectFPExt(const Instruction *I);
   bool selectFPTrunc(const Instruction *I);
   bool selectFPToInt(const Instruction *I, bool IsSigned);
   bool selectRet(const Instruction *I);
   bool selectTrunc(const Instruction *I);
   bool selectIntExt(const Instruction *I);
   bool selectShift(const Instruction *I);
if (!SrcReg)
     return false;
 
   unsigned DestReg = createResultReg(&Mips::AFGR64RegClass);
   emitInst(Mips::CVT_D32_S, DestReg).addReg(SrcReg);
   updateValueMap(I, DestReg);
   return true;
 }
 
+bool MipsFastISel::selectSelect(const Instruction *I) {
+  assert(isa<SelectInst>(I) && "Expected a select instruction.");
+
+  MVT VT;
+  if (!isTypeSupported(I->getType(), VT))
+    return false;
+
+  unsigned CondMovOpc;
+  const TargetRegisterClass *RC;
+
+  if (VT.isInteger() && !VT.isVector() && VT.getSizeInBits() <= 32) {
+    CondMovOpc = Mips::MOVN_I_I;
+    RC = &Mips::GPR32RegClass;
+  } else if (VT == MVT::f32) {
+    CondMovOpc = Mips::MOVN_I_S;
+    RC = &Mips::FGR32RegClass;
+  } else if (VT == MVT::f64) {
+    CondMovOpc = Mips::MOVN_I_D32;
+    RC = &Mips::AFGR64RegClass;
+  } else
+    return false;
+
+  const SelectInst *SI = cast<SelectInst>(I);
+  const Value *Cond = SI->getCondition();
+  unsigned Src1Reg = getRegForValue(SI->getTrueValue());
+  unsigned Src2Reg = getRegForValue(SI->getFalseValue());
+  unsigned CondReg = getRegForValue(Cond);
+
+  if (!Src1Reg || !Src2Reg || !CondReg)
+    return false;
+
+  unsigned ResultReg = createResultReg(RC);
+  unsigned TempReg = createResultReg(RC);
+
+  if (!ResultReg || !TempReg)
+    return false;
+
+  emitInst(TargetOpcode::COPY, TempReg).addReg(Src2Reg);
+  emitInst(CondMovOpc, ResultReg)
+    .addReg(Src1Reg).addReg(CondReg).addReg(TempReg);
+  updateValueMap(I, ResultReg);
+  return true;
+}
+
 // Attempt to fast-select a floating-point truncate instruction.
 bool MipsFastISel::selectFPTrunc(const Instruction *I) {
   if (UnsupportedFPMode)
     return false;
   Value *Src = I->getOperand(0);
   EVT SrcVT = TLI.getValueType(Src->getType(), true);
   EVT DestVT = TLI.getValueType(I->getType(), true);
 
case Instruction::FPExt:
     return selectFPExt(I);
   case Instruction::FPToSI:
     return selectFPToInt(I, /*isSigned*/ true);
   case Instruction::FPToUI:
     return selectFPToInt(I, /*isSigned*/ false);
   case Instruction::ICmp:
   case Instruction::FCmp:
     return selectCmp(I);
+  case Instruction::Select:
+    return selectSelect(I);
   }
   return false;
 }
 
 unsigned MipsFastISel::getRegEnsuringSimpleIntegerWidening(const Value *V,
                                                            bool IsUnsigned) {
   unsigned VReg = getRegForValue(V);
   if (VReg == 0)

llvm/trunk/test/CodeGen/Mips/Fast-ISel/sel1.ll

+; RUN: llc < %s -march=mipsel -mcpu=mips32r2 -O2 -relocation-model=pic \
+; RUN:          -fast-isel -mips-fast-isel -fast-isel-abort=1 | FileCheck %s
+
+define i1 @sel_i1(i1 %j, i1 %k, i1 %l) {
+entry:
+  ; CHECK-LABEL:  sel_i1:
+
+  ; FIXME: The following instruction is redundant.
+  ; CHECK:            xor     $[[T0:[0-9]+]], $4, $zero
+  ; CHECK-NEXT:       sltu    $[[T1:[0-9]+]], $zero, $[[T0]]
+  ; CHECK-NEXT:       movn    $6, $5, $[[T1]]
+  ; CHECK:            move    $2, $6
+  %cond = icmp ne i1 %j, 0
+  %res = select i1 %cond, i1 %k, i1 %l
+  ret i1 %res
+}
+
+define i8 @sel_i8(i8 %j, i8 %k, i8 %l) {
+entry:
+  ; CHECK-LABEL:  sel_i8:
+
+  ; CHECK-DAG:        seb     $[[T0:[0-9]+]], $4
+  ; FIXME: The following 2 instructions are redundant.
+  ; CHECK-DAG:        seb     $[[T1:[0-9]+]], $zero
+  ; CHECK:            xor     $[[T2:[0-9]+]], $[[T0]], $[[T1]]
+  ; CHECK-NEXT:       sltu    $[[T3:[0-9]+]], $zero, $[[T2]]
+  ; CHECK-NEXT:       movn    $6, $5, $[[T3]]
+  ; CHECK:            move    $2, $6
+  %cond = icmp ne i8 %j, 0
+  %res = select i1 %cond, i8 %k, i8 %l
+  ret i8 %res
+}
+
+define i16 @sel_i16(i16 %j, i16 %k, i16 %l) {
+entry:
+  ; CHECK-LABEL:  sel_i16:
+
+  ; CHECK-DAG:        seh     $[[T0:[0-9]+]], $4
+  ; FIXME: The following 2 instructions are redundant.
+  ; CHECK-DAG:        seh     $[[T1:[0-9]+]], $zero
+  ; CHECK:            xor     $[[T2:[0-9]+]], $[[T0]], $[[T1]]
+  ; CHECK-NEXT:       sltu    $[[T3:[0-9]+]], $zero, $[[T2]]
+  ; CHECK-NEXT:       movn    $6, $5, $[[T3]]
+  ; CHECK:            move    $2, $6
+  %cond = icmp ne i16 %j, 0
+  %res = select i1 %cond, i16 %k, i16 %l
+  ret i16 %res
+}
+
+define i32 @sel_i32(i32 %j, i32 %k, i32 %l) {
+entry:
+  ; CHECK-LABEL:  sel_i32:
+
+  ; FIXME: The following instruction is redundant.
+  ; CHECK:            xor     $[[T0:[0-9]+]], $4, $zero
+  ; CHECK-NEXT:       sltu    $[[T1:[0-9]+]], $zero, $[[T0]]
+  ; CHECK-NEXT:       movn    $6, $5, $[[T1]]
+  ; CHECK:            move    $2, $6
+  %cond = icmp ne i32 %j, 0
+  %res = select i1 %cond, i32 %k, i32 %l
+  ret i32 %res
+}
+
+define float @sel_float(i32 %j, float %k, float %l) {
+entry:
+  ; CHECK-LABEL:  sel_float:
+
+  ; CHECK-DAG:        mtc1    $6, $f0
+  ; CHECK-DAG:        mtc1    $5, $f1
+  ; CHECK-DAG:        xor     $[[T0:[0-9]+]], $4, $zero
+  ; CHECK:            sltu    $[[T1:[0-9]+]], $zero, $[[T0]]
+  ; CHECK:            movn.s  $f0, $f1, $[[T1]]
+  %cond = icmp ne i32 %j, 0
+  %res = select i1 %cond, float %k, float %l
+  ret float %res
+}
+
+define double @sel_double(i32 %j, double %k, double %l) {
+entry:
+  ; CHECK-LABEL:  sel_double:
+
+  ; CHECK-DAG:        mtc1    $6, $f2
+  ; CHECK-DAG:        mthc1   $7, $f2
+  ; CHECK-DAG:        ldc1    $f0, 16($sp)
+  ; CHECK-DAG:        xor     $[[T0:[0-9]+]], $4, $zero
+  ; CHECK:            sltu    $[[T1:[0-9]+]], $zero, $[[T0]]
+  ; CHECK:            movn.d  $f0, $f2, $[[T1]]
+  %cond = icmp ne i32 %j, 0
+  %res = select i1 %cond, double %k, double %l
+  ret double %res
+}