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

src/IceTargetLoweringMIPS32.cpp

Index: src/IceTargetLoweringMIPS32.cpp
diff --git a/src/IceTargetLoweringMIPS32.cpp b/src/IceTargetLoweringMIPS32.cpp
index 622af79a9a53a7036ce894bdd154db04a5cc2aad..31ba132b1a00e636dd5ac040b1f8f0e6a50af205 100644
--- a/src/IceTargetLoweringMIPS32.cpp
+++ b/src/IceTargetLoweringMIPS32.cpp
@@ -2731,6 +2731,20 @@ void TargetMIPS32::lowerRet(const InstRet *Instr) {
   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:
@@ -2757,7 +2771,44 @@ void TargetMIPS32::lowerRet(const InstRet *Instr) {
 }
 
 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) {