[SubZero] Implement GP to/from FP moves for MIPS

src/IceInstMIPS32.cpp

Index: src/IceInstMIPS32.cpp
diff --git a/src/IceInstMIPS32.cpp b/src/IceInstMIPS32.cpp
index e7485b915457732924a7a0ed388ee088e7fef2f8..65c831d0ddfdd8d6f6a53e027a7eb92c954d10ef 100644
--- a/src/IceInstMIPS32.cpp
+++ b/src/IceInstMIPS32.cpp
@@ -713,27 +713,40 @@ void InstMIPS32Mov::emitSingleDestSingleSource(const Cfg *Func) const {
 
   // reg to reg
   if (DestIsReg && SrcIsReg) {
-    switch (Dest->getType()) {
-    case IceType_f32:
-      ActualOpcode = "mov.s";
-      break;
-    case IceType_f64:
-      ActualOpcode = "mov.d";
-      break;
-    case IceType_i1:
-    case IceType_i8:
-    case IceType_i16:
-    case IceType_i32:
-      Str << "\t"
-          << "move"
-          << "\t";
-      getDest()->emit(Func);
-      Str << ", ";
-      getSrc(0)->emit(Func);
-      return;
-    default:
-      UnimplementedError(getFlags());
-      return;
+    const Type DstType = Dest->getType();
+    const Type SrcType = Src->getType();
+
+    // move GP to/from FP
+    if (DstType != SrcType) {
+      if (isScalarFloatingType(DstType) == true) {

[Jim Stichnoth, 2016/09/07 15:39:31]

remove the redundant "== true"

[jaydeep.patil, 2016/09/13 06:18:36]

Done.

+        Str << "\t"
+               "mtc1"
+               "\t";
+        getSrc(0)->emit(Func);
+        Str << ", ";
+        getDest()->emit(Func);
+        return;
+      } else {

[Jim Stichnoth, 2016/09/07 15:39:31]

http://llvm.org/docs/CodingStandards.html#don-t-use-else-after-a-return

[jaydeep.patil, 2016/09/13 06:18:36]

Done.

+        ActualOpcode = "mfc1";
+      }
+    } else {
+      switch (Dest->getType()) {
+      case IceType_f32:
+        ActualOpcode = "mov.s";
+        break;
+      case IceType_f64:
+        ActualOpcode = "mov.d";
+        break;
+      case IceType_i1:
+      case IceType_i8:
+      case IceType_i16:
+      case IceType_i32:
+        ActualOpcode = "move";

[obucinac, 2016/09/08 09:52:01]

As adviced earlier by Simon Dardis, we should use OR here.

Related links:

https://sourceware.org/ml/binutils/2015-07/msg00376.html

http://reviews.llvm.org/rL244579

Uncommited (but not commitable anymore) change:
https://codereview.chromium.org/2122043002/patch/40001/50001



Simons advice:

Your patch for supporting different data-types for Subzero has one small nit in
it:

+    case IceType_i1:
+    case IceType_i8:
+    case IceType_i16:
+    case IceType_i32:
+      Str << "\t"
+             "add"
              "\t";
-      getSrc(0)->emit(Func);
-      Str << ", ";
       getDest()->emit(Func);
+      Str << ", $zero, ";

That "add" should be an "or" instead. We re-encoded that macro to use 'or'
instead of 'add'
for all MIPS ISAs. See https://sourceware.org/ml/binutils/2015-07/msg00376.html
and
http://reviews.llvm.org/rL244579 .

The background for this is that 'or' is better than 'add' for 64bit cores
running O32 code as
'add' is required to sign-extend its result whereas 'or' doesn't.

Thanks,
Simon

[jaydeep.patil, 2016/09/13 06:18:36]

This change should be done in AssemblerMIPS32::move instead of
InstMIPS32Mov::emitSingleDestSingleSource. 'move' is a pseudo instruction, let
the assembler encode it as 'or' instead of 'addu'.
We will submit a separate patch for this.

+        break;
+      default:
+        UnimplementedError(getFlags());
+        return;
+      }
     }
 
     assert(ActualOpcode);