Subzero, MIPS32: Handling floating point instructions fadd, fsub, fmul, fdiv

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 392 matching lines @@
   Context.setInsertPoint(Context.getCur());
   for (SizeT I = 0, E = Args.size(); I < E; ++I) {
     Variable *Arg = Args[I];
     Type Ty = Arg->getType();
     // TODO(rkotler): handle float/vector types.
     if (isVectorType(Ty)) {
       UnimplementedError(getFlags());
       continue;
     }
     if (isFloatingType(Ty)) {
       UnimplementedError(getFlags());

[Jim Stichnoth, 2016/06/01 13:52:13]

When I run the updated lit test, I'm getting a failure due to liveness
validation failure.  I suspect this part of the code is responsible, because
this is the first unimplemented error seen when I remove the
"--skip-unimplemented" flag.

[obucinac, 2016/06/01 14:21:16]

This is because calling convention is still not in place, and handling fp
arguments (loading it to fp register) is missing.

I believe this is dependency:
https://codereview.chromium.org/2021033002/

[Jim Stichnoth, 2016/06/01 21:22:08]

Hmm, I get the same unimplemented error in this place, even with that patch
applied on top of this one.

I looked into some options for creating float/double operands that could be used
instead of function arguments:
  - %add = fadd float undef, undef
  - %a = load float, float* %addr, align 4
  - cast/convert something to float

Unfortunately, in all these cases it looks like we're still missing a bit of
functionality elsewhere.

We can't really check in code that breaks the tests like this CL currently does.
 I think the CL is otherwise good, and the tests are good, so you could consider
temporarily disabling the tests.

My favorite way of temporarily disabling a lit test is to change "RUN:" to
"RUIN:" .

       continue;
     }
     if (Ty == IceType_i64) {
       if (NumGPRRegsUsed >= MIPS32_MAX_GPR_ARG)
         continue;
       auto RegLo = RegNumT::fixme(RegMIPS32::Reg_A0 + NumGPRRegsUsed);
       auto RegHi = RegNumT::fixme(RegLo + 1);
       ++NumGPRRegsUsed;
       // Always start i64 registers at an even register, so this may end
       // up padding away a register.
@@ skipping 259 matching lines @@
   Operand *Src0 = legalizeUndef(Instr->getSrc(0));
   Operand *Src1 = legalizeUndef(Instr->getSrc(1));
   if (DestTy == IceType_i64) {
     lowerInt64Arithmetic(Instr, Instr->getDest(), Src0, Src1);
     return;
   }
   if (isVectorType(Dest->getType())) {
     UnimplementedLoweringError(this, Instr);
     return;
   }
-  switch (Instr->getOp()) {
-  default:
-    break;
-  case InstArithmetic::Fadd:
-  case InstArithmetic::Fsub:
-  case InstArithmetic::Fmul:
-  case InstArithmetic::Fdiv:
-  case InstArithmetic::Frem:
-    UnimplementedLoweringError(this, Instr);
-    return;
-  }
 
   // At this point Dest->getType() is non-i64 scalar
 
   Variable *T = makeReg(Dest->getType());
   Variable *Src0R = legalizeToReg(Src0);
   Variable *Src1R = legalizeToReg(Src1);
 
   switch (Instr->getOp()) {
   case InstArithmetic::_num:
     break;
@@ skipping 58 matching lines @@
     _mov(Dest, T);
     return;
   }
   case InstArithmetic::Srem: {
     auto *T_Zero = I32Reg(RegMIPS32::Reg_ZERO);
     _div(T_Zero, Src0R, Src1R);
     _mfhi(T, T_Zero);
     _mov(Dest, T);
     return;
   }
-  case InstArithmetic::Fadd:
+  case InstArithmetic::Fadd: {
+    if (DestTy == IceType_f32) {
+      _add_s(T, Src0R, Src1R);
+      _mov_s(Dest, T);
+      return;
+    }
+    if (DestTy == IceType_f64) {
+      _add_d(T, Src0R, Src1R);
+      _mov_d(Dest, T);
+      return;
+    }
     break;
+  }
   case InstArithmetic::Fsub:
+    if (DestTy == IceType_f32) {
+      _sub_s(T, Src0R, Src1R);
+      _mov_s(Dest, T);
+      return;
+    }
+    if (DestTy == IceType_f64) {
+      _sub_d(T, Src0R, Src1R);
+      _mov_d(Dest, T);
+      return;
+    }
     break;
   case InstArithmetic::Fmul:
+    if (DestTy == IceType_f32) {
+      _mul_s(T, Src0R, Src1R);
+      _mov_s(Dest, T);
+      return;
+    }
+    if (DestTy == IceType_f64) {
+      _mul_d(T, Src0R, Src1R);
+      _mov_d(Dest, T);
+      return;
+    }
     break;
   case InstArithmetic::Fdiv:
+    if (DestTy == IceType_f32) {
+      _div_s(T, Src0R, Src1R);
+      _mov_s(Dest, T);
+      return;
+    }
+    if (DestTy == IceType_f64) {
+      _div_d(T, Src0R, Src1R);
+      _mov_d(Dest, T);
+      return;
+    }
     break;
   case InstArithmetic::Frem:

[Jim Stichnoth, 2016/06/01 13:52:13]

For frem, all the other targets create a call to the frem_f32 or frem_f64
runtime helper, you may want to take this same approach to keep things simple. 
In practice, there don't seem to be a lot of frem calls so there's not a
performance issue.

[obucinac, 2016/06/01 14:21:16]

Acknowledged.

     break;
   }
   UnimplementedLoweringError(this, Instr);
 }
 
 void TargetMIPS32::lowerAssign(const InstAssign *Instr) {
   Variable *Dest = Instr->getDest();
   Operand *Src0 = Instr->getSrc(0);
   assert(Dest->getType() == Src0->getType());
   if (Dest->getType() == IceType_i64) {
@@ skipping 867 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