​MIPS: Improve Cvt_d_uw on mips32.

src/mips/macro-assembler-mips.cc

 
 // Copyright 2012 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 <limits.h>  // For LONG_MIN, LONG_MAX.
 
 #if V8_TARGET_ARCH_MIPS
 
 #include "src/base/bits.h"
@@ skipping 1252 matching lines @@
     nor(at, at, zero_reg);
     and_(at, rt, at);
     or_(rt, t8, at);
   }
 }
 
 
 void MacroAssembler::Cvt_d_uw(FPURegister fd,
                               FPURegister fs,
                               FPURegister scratch) {
-  // Move the data from fs to t8.
-  mfc1(t8, fs);
-  Cvt_d_uw(fd, t8, scratch);
+  if (IsFp64Mode()) {
+    // on FP64Mode we can simple do convertion from long

[paul.l..., 2015/11/18 01:49:13]

Please start comments with capital letter, and end with a period.

[Alan Li, 2015/11/18 17:07:03]

Done.

+    cvt_d_l(fd, fs);

[Alan Li, 2015/11/18 02:53:21]

On FP64Mode, can we assume that fs.high() is filled with 0x0 in this case?

[Alan Li, 2015/11/18 17:07:03]

Acknowledged.

+  } else {
+    // Move the data from fs to t8.

[paul.l..., 2015/11/18 01:49:13]

There is no need for this comment, it just explains what the instruction does,
so it's redundant.
Comments should be used when the intent might not be clear. 

[Alan Li, 2015/11/18 17:07:03]

Done.

+    mfc1(t8, fs);
+    Cvt_d_uw(fd, t8, scratch, fs);
+  }
 }
 
 
-void MacroAssembler::Cvt_d_uw(FPURegister fd,
-                              Register rs,
-                              FPURegister scratch) {
+void MacroAssembler::Cvt_d_uw(FPURegister fd, Register rs, FPURegister scratch,
+                              FPURegister fs) {
   // Convert rs to a FP value in fd (and fd + 1).
   // We do this by converting rs minus the MSB to avoid sign conversion,
   // then adding 2^31 to the result (if needed).

[paul.l..., 2015/11/18 01:49:13]

This comment block is stale, you changed the function, it no longer works this
way. Please update the comment.
You are doing some tricky stuff with treating an unsigned number in range
0x8000.0000 to 0xffff.ffff as a signed (negative) number, converting that to FP,
then adding 2^32 to it. That is not so obvious, and should be explained by
comment.

[Alan Li, 2015/11/18 17:07:03]

Done.

 
   DCHECK(!fd.is(scratch));
   DCHECK(!rs.is(t9));
   DCHECK(!rs.is(at));
 
-  // Save rs's MSB to t9.
-  Ext(t9, rs, 31, 1);
-  // Remove rs's MSB.
-  Ext(at, rs, 0, 31);
-  // Move the result to fd.
-  mtc1(at, fd);
+  Label msb_clear, conversion_done;
+  Branch(&msb_clear, ge, rs, Operand(zero_reg));

[paul.l..., 2015/11/18 01:49:13]

you can use delay slot here to save an instruction. The li() below is a single
instruction (lui), and if you take the branch it doesn't matter if you overwrite
'at'.

If you do this, I would recommend writing it as 'lui(at, 0x41f0);  // FP value:
2^32, in delay slot.

[Alan Li, 2015/11/18 17:07:03]

Done.

 
-  // Convert fd to a real FP value.
-  cvt_d_w(fd, fd);
-
-  Label conversion_done;
-
-  // If rs's MSB was 0, it's done.
-  // Otherwise we need to add that to the FP register.
-  Branch(&conversion_done, eq, t9, Operand(zero_reg));
-
-  // Load 2^31 into f20 as its float representation.
-  li(at, 0x41E00000);
+  li(at, 0x41f00000);

[paul.l..., 2015/11/18 01:49:13]

This should have a comment, about FP value of 2^32

[Alan Li, 2015/11/18 17:07:03]

Done.

   mtc1(zero_reg, scratch);
   Mthc1(at, scratch);
-  // Add it to fd.
+
+  if (fs.is(no_freg)) {
+    mtc1(rs, fd);
+    cvt_d_w(fd, fd);
+  } else {
+    cvt_d_w(fd, fs);
+  }
+
+  Branch(USE_DELAY_SLOT, &conversion_done);
   add_d(fd, fd, scratch);

[paul.l..., 2015/11/18 01:49:13]

Nice use of delay slot!

[Alan Li, 2015/11/18 17:07:03]

Acknowledged.

 
+  bind(&msb_clear);
+  if (fs.is(no_freg)) {
+    mtc1(rs, fd);
+    cvt_d_w(fd, fd);
+  } else {
+    cvt_d_w(fd, fs);
+  }
+
   bind(&conversion_done);
 }
 
 
 void MacroAssembler::Trunc_uw_d(FPURegister fd,
                                 FPURegister fs,
                                 FPURegister scratch) {
   Trunc_uw_d(fs, t8, scratch);
   mtc1(t8, fd);
 }
@@ skipping 4531 matching lines @@
   if (mag.shift > 0) sra(result, result, mag.shift);
   srl(at, dividend, 31);
   Addu(result, result, Operand(at));
 }
 
 
 }  // namespace internal
 }  // namespace v8
 
 #endif  // V8_TARGET_ARCH_MIPS