Subzero, MIPS32: lowerSelect for i1, i8, i16, i32, f32, f64

src/IceTargetLoweringMIPS32.cpp

 //
 //                        The Subzero Code Generator
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 ///
 /// \file
 /// \brief Implements the TargetLoweringMIPS32 class, which consists almost
@@ skipping 2713 matching lines @@
 
 void TargetMIPS32::lowerPhi(const InstPhi * /*Instr*/) {
   Func->setError("Phi found in regular instruction list");
 }
 
 void TargetMIPS32::lowerRet(const InstRet *Instr) {
   Variable *Reg = nullptr;
   if (Instr->hasRetValue()) {
     Operand *Src0 = Instr->getRetValue();
     switch (Src0->getType()) {
+    case IceType_f32: {

[Jim Stichnoth, 2016/09/13 13:56:22]

What happens with an instruction like this?

  ret float 1.0
  ret double 1.0

Before, the code would do an UnimplementedLoweringError, which is fine.

Now, it looks like it will fall through to _ret(Reg_RA) which is surely wrong.

[obucinac, 2016/09/13 17:26:18]

Done.

+      if (auto *Src0V = llvm::dyn_cast<Variable>(Src0)) {
+        Reg = makeReg(Src0V->getType(), RegMIPS32::Reg_F0);
+        _mov(Reg, Src0V);
+      }
+      break;
+    }
+    case IceType_f64: {
+      if (auto *Src0V = llvm::dyn_cast<Variable>(Src0)) {
+        Reg = makeReg(Src0V->getType(), RegMIPS32::Reg_F0F1);
+        _mov(Reg, Src0V);
+      }
+      break;
+    }
     case IceType_i1:
     case IceType_i8:
     case IceType_i16:
     case IceType_i32: {
       // Reg = legalizeToReg(Src0, RegMIPS32::Reg_V0);
       Operand *Src0F = legalize(Src0, Legal_Reg);
       Reg = makeReg(Src0F->getType(), RegMIPS32::Reg_V0);
       _mov(Reg, Src0F);
       break;
     }
     case IceType_i64: {
       Src0 = legalizeUndef(Src0);
       Variable *R0 = legalizeToReg(loOperand(Src0), RegMIPS32::Reg_V0);
       Variable *R1 = legalizeToReg(hiOperand(Src0), RegMIPS32::Reg_V1);
       Reg = R0;
       Context.insert<InstFakeUse>(R1);
       break;
     }
     default:
       UnimplementedLoweringError(this, Instr);
     }
   }
   _ret(getPhysicalRegister(RegMIPS32::Reg_RA), Reg);
 }
 
 void TargetMIPS32::lowerSelect(const InstSelect *Instr) {
-  UnimplementedLoweringError(this, Instr);
+  Variable *Dest = Instr->getDest();
+  Type DestTy = Dest->getType();

[Jim Stichnoth, 2016/09/13 13:56:22]

const Type

[obucinac, 2016/09/13 17:26:18]

Done.

+
+  if (DestTy == IceType_i64 || isVectorType(DestTy)) {
+    UnimplementedLoweringError(this, Instr);
+    return;
+  }
+
+  Variable *DestR = legalizeToReg(Dest);
+  Variable *SrcTR = legalizeToReg(Instr->getTrueOperand());
+  Variable *SrcFR = legalizeToReg(Instr->getFalseOperand());
+
+  Variable *ConditionR = legalizeToReg(Instr->getCondition());
+
+  assert(Instr->getCondition()->getType() == IceType_i1);
+
+  switch (DestTy) {
+  case IceType_i1:
+  case IceType_i8:
+  case IceType_i16:
+  case IceType_i32:
+    _movn(SrcFR, SrcTR, ConditionR);

[Jim Stichnoth, 2016/09/13 13:56:22]

Have you tried running with "pnacl-sz -O2"?

I suspect these _movn instructions might trigger a liveness error, because SrcFR
is being redefined here (after being first defined above by copying the false
operand into a register).

If this happens, the way to deal with it is to add:
  _set_dest_redefined();

[obucinac, 2016/09/13 17:26:18]

Checked, no problem here. O2 test added.

+    _mov(DestR, SrcFR);
+    _mov(Dest, DestR);

[Jim Stichnoth, 2016/09/13 13:56:22]

Can you skip the previous _mov and just do
  _mov(Dest, SrcFR);
?

[obucinac, 2016/09/13 17:26:18]

No. With previous changes, _mov handles only reg to reg copying. Without two
_mov we do not cover the case when destination is memory.

+    break;
+  case IceType_f32:
+    _movn_s(SrcFR, SrcTR, ConditionR);
+    _mov(DestR, SrcFR);
+    _mov(Dest, DestR);
+    break;
+  case IceType_f64:
+    _movn_d(SrcFR, SrcTR, ConditionR);
+    _mov(DestR, SrcFR);
+    _mov(Dest, DestR);
+    break;
+  default:
+    UnimplementedLoweringError(this, Instr);
+  }
 }
 
 void TargetMIPS32::lowerShuffleVector(const InstShuffleVector *Instr) {
   UnimplementedLoweringError(this, Instr);
 }
 
 void TargetMIPS32::lowerStore(const InstStore *Instr) {
   Operand *Value = Instr->getData();
   Operand *Addr = Instr->getAddr();
   OperandMIPS32Mem *NewAddr = formMemoryOperand(Addr, Value->getType());
@@ skipping 360 matching lines @@
   Str << "\t.set\t"
       << "nomips16\n";
 }
 
 SmallBitVector TargetMIPS32::TypeToRegisterSet[RCMIPS32_NUM];
 SmallBitVector TargetMIPS32::TypeToRegisterSetUnfiltered[RCMIPS32_NUM];
 SmallBitVector TargetMIPS32::RegisterAliases[RegMIPS32::Reg_NUM];
 
 } // end of namespace MIPS32
 } // end of namespace Ice