MIPS: [turbofan] Properly implement Float64/32 Min/Max instructions.

src/compiler/mips/code-generator-mips.cc

 // Copyright 2014 the V8 project authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "src/compiler/code-generator.h"
 #include "src/compiler/code-generator-impl.h"
 #include "src/compiler/gap-resolver.h"
 #include "src/compiler/node-matchers.h"
 #include "src/compiler/osr.h"
 #include "src/mips/macro-assembler-mips.h"
@@ skipping 703 matching lines @@
       break;
     }
     case kMipsFloat64RoundTruncate: {
       ASSEMBLE_ROUND_DOUBLE_TO_DOUBLE(trunc_l_d, Truncate);
       break;
     }
     case kMipsFloat64RoundUp: {
       ASSEMBLE_ROUND_DOUBLE_TO_DOUBLE(ceil_l_d, Ceil);
       break;
     }
+    case kMipsFloat64Max: {
+      // (b < a) ? a : b
+      if (IsMipsArchVariant(kMips32r6)) {
+        __ cmp(OLT, L, kScratchDoubleReg, i.InputDoubleRegister(0),

[paul.l..., 2015/11/02 22:35:11]

You're putting the result of cmp in scratch, but due to semantics of sel_d() it
needs to be in output register.

[dusan.milosavljevic, 2015/11/03 01:51:05]

Done.

+               i.InputDoubleRegister(1));
+        __ sel_d(i.OutputSingleRegister(), i.InputDoubleRegister(1),
+                 kScratchDoubleReg);

[paul.l..., 2015/11/02 22:35:11]

s/b: sel_d(i.OutputDoubleRegister(),
i.InputDoubleRegister(0),i.InputDoubleRegister(1))

from manual: FPR[fd] 􏰀= FPR[fd].bit0 ? FPR[ft] : FPR[fs]

I don't see how this is working, unless we have insufficient test coverage.

[dusan.milosavljevic, 2015/11/03 01:51:05]

Done.

Different instruction from other sel. instruction, i assumed they are similar,
but this one is not.

+      } else {
+        __ c(OLT, D, i.InputDoubleRegister(0), i.InputDoubleRegister(1));
+        // Left operand is result, passthrough if false.
+        __ movt_d(i.OutputSingleRegister(), i.InputDoubleRegister(1));

[paul.l..., 2015/11/02 22:35:12]

All the Float64 output regs are show as OutputSingleRegister(), but should be
OutputDoubleRegister().

I do realize they are aliases, but we should use them consistently.

[dusan.milosavljevic, 2015/11/03 01:51:05]

Done.

+      }
+      break;
+    }
+    case kMipsFloat64Min: {
+      // (a < b) ? a : b
+      if (IsMipsArchVariant(kMips32r6)) {
+        __ cmp(OLT, L, kScratchDoubleReg, i.InputDoubleRegister(0),

[paul.l..., 2015/11/02 22:35:11]

nit: we have asm aliases for cmp_d() and cmp_s(), you should use those here, and
below. Using the raw cmp() with L & W types is correct per the encodings, but
its a cognitive dissonance, since every other usage of L & W are for ints, and D
& S are the common usage for floats. (above, here, and below)

[dusan.milosavljevic, 2015/11/03 01:51:05]

Done.

+               i.InputDoubleRegister(1));
+        __ sel_d(i.OutputSingleRegister(), i.InputDoubleRegister(1),
+                 kScratchDoubleReg);
+      } else {
+        __ c(OLT, D, i.InputDoubleRegister(1), i.InputDoubleRegister(0));
+        // Right operand is result, passthrough if false.
+        __ movt_d(i.OutputSingleRegister(), i.InputDoubleRegister(1));
+      }
+      break;
+    }
+    case kMipsFloat32Max: {
+      // (b < a) ? a : b
+      if (IsMipsArchVariant(kMips32r6)) {
+        __ cmp(OLT, W, kScratchDoubleReg, i.InputDoubleRegister(0),
+               i.InputDoubleRegister(1));
+        __ sel_s(i.OutputSingleRegister(), i.InputDoubleRegister(1),
+                 kScratchDoubleReg);
+      } else {
+        __ c(OLT, S, i.InputDoubleRegister(0), i.InputDoubleRegister(1));
+        // Left operand is result, passthrough if false.
+        __ movt_s(i.OutputSingleRegister(), i.InputDoubleRegister(1));
+      }
+      break;
+    }
+    case kMipsFloat32Min: {
+      // (a < b) ? a : b
+      if (IsMipsArchVariant(kMips32r6)) {
+        __ cmp(OLT, W, kScratchDoubleReg, i.InputDoubleRegister(0),
+               i.InputDoubleRegister(1));
+        __ sel_d(i.OutputSingleRegister(), i.InputDoubleRegister(1),
+                 kScratchDoubleReg);
+      } else {
+        __ c(OLT, S, i.InputDoubleRegister(1), i.InputDoubleRegister(0));
+        // Right operand is result, passthrough if false.
+        __ movt_s(i.OutputSingleRegister(), i.InputDoubleRegister(1));
+      }
+      break;
+    }
     case kMipsCvtSD: {
       __ cvt_s_d(i.OutputSingleRegister(), i.InputDoubleRegister(0));
       break;
     }
     case kMipsCvtDS: {
       __ cvt_d_s(i.OutputDoubleRegister(), i.InputSingleRegister(0));
       break;
     }
     case kMipsCvtDW: {
       FPURegister scratch = kScratchDoubleReg;
@@ skipping 726 matching lines @@
       padding_size -= v8::internal::Assembler::kInstrSize;
     }
   }
 }
 
 #undef __
 
 }  // namespace compiler
 }  // namespace internal
 }  // namespace v8