​MIPS: Improve Cvt_d_uw on mips32.

src/mips/macro-assembler-mips.cc

Index: src/mips/macro-assembler-mips.cc
diff --git a/src/mips/macro-assembler-mips.cc b/src/mips/macro-assembler-mips.cc
index 272f8147b94185aac05ed5dd75d18940e2fb7e03..ecb50cc487e59d597a5801c0057f85f10d1fe124 100644
--- a/src/mips/macro-assembler-mips.cc
+++ b/src/mips/macro-assembler-mips.cc
@@ -1270,15 +1270,19 @@ void MacroAssembler::Ins(Register rt,
 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.

@@ -1287,29 +1291,31 @@ void MacroAssembler::Cvt_d_uw(FPURegister fd,
   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);
-
-  // 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));
+  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.

 
-  // 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);
 }