A64 support for DoubleToIStub (truncating).

src/a64/code-stubs-a64.cc

 // Copyright 2013 the V8 project authors. All rights reserved.
 // Redistribution and use in source and binary forms, with or without
 // modification, are permitted provided that the following conditions are
 // met:
 //
 //     * Redistributions of source code must retain the above copyright
 //       notice, this list of conditions and the following disclaimer.
 //     * Redistributions in binary form must reproduce the above
 //       copyright notice, this list of conditions and the following
 //       disclaimer in the documentation and/or other materials provided
@@ skipping 518 matching lines @@
       __ Push(descriptor->register_params_[i]);
     }
     ExternalReference miss = descriptor->miss_handler();
     __ CallExternalReference(miss, descriptor->register_param_count_);
   }
 
   __ Ret();
 }
 
 
+void DoubleToIStub::Generate(MacroAssembler* masm) {

[jbramley, 2014/02/13 14:20:14]

It seems that this stub doesn't always truncate on other architectures. We
should ASSERT(is_truncating()) somewhere.

[rmcilroy, 2014/02/14 12:45:07]

Done (also on arm). Looks like the is_truncating() option isn't supported by any
arches, we should maybe just drop the option...

[jbramley, 2014/02/14 12:57:38]

If we do, we should probably rename the stub too, but that's something to worry
about later.

+  Label done;
+  Register input = source();
+  Register result = destination();
+
+  ASSERT(result.Is64Bits());
+  ASSERT(jssp.Is(masm->StackPointer()));
+
+  int double_offset = offset();
+  // Account for saved regs if input is sp.
+  if (input.is(jssp)) double_offset += 2 * kPointerSize;

[jbramley, 2014/02/13 14:20:14]

Do this near Push(scratch1, scratch2) so it's clear what it's for.

[rmcilroy, 2014/02/14 12:45:07]

Done.

+
+  DoubleRegister double_scratch = d0;  // only used if !skip_fastpath()

[jbramley, 2014/02/13 14:20:14]

In that case, put it in the !skip_fastpath() scope.

[rmcilroy, 2014/02/14 12:45:07]

I can't - I need to Pop it at the end of the function.

+  Register scratch1 = GetRegisterThatIsNotOneOf(input, result);
+  Register scratch2 =
+      GetRegisterThatIsNotOneOf(input, result, scratch1);
+
+  __ Push(scratch1, scratch2);

[jbramley, 2014/02/13 14:20:14]

This is probably the only stub that preserves its registers. It took us a little
while to realise that this didn't need to be marked as a call. Could you add a
comment to the call site please?

Also, it would be more efficient to ask for temp registers than to
unconditionally preserve the scratch registers. Is there any way to arrange
that? Even using DefineFixed would be better than unconditionally preserving
them on the stack.

[rmcilroy, 2014/02/14 12:45:07]

I'm not sure what you mean here? What would you mean by "marked as a call" - it
is called with CallStub, which typically doesn't respect caller/callee saved
register call semantics.  As far as I can tell RecordWriteStub does the same
thing (for scratch1_/scratch2_ and it doesn't have any special "marked as call"
or comment that I can see.
 
If we allocated temp registers then we would need to allocate these every time
MacroAssembler::TruncateDoubleToI() was called, even though they would only
actually be used if we had to call into the stub (which is fairly infrequent
since most situations are successful with the inline code).  It's a trade off
between additional register pressure (for every TruncateDoubleToI) verses
additional push/pop overhead, but only in situations where we need to call out
to the stub.   When I did this for Arm there was no noticeable overhead increase
caused by the additions of the push/pops so I would prefer to do that here too
unless you strongly disagree.

[jbramley, 2014/02/14 12:57:38]

At the moment, MarkAsCall preserves every live value (if I remember correctly),
even callee-saved values, because the call might do a GC.

Our stubs don't really have an ABI; we tend to be cautious and assume that they
can clobber anything except the stack. We inherited this from ARM, but we'd like
to improve this later.

It's perfectly Ok to define a stub that explicitly doesn't clobber any
registers, but it's unusual (for A64) so it needs a comment at the call site. If
I see CallStub, I normally think "clobber everything".

RecordWriteStub is a bit weird and I think I have some TODOs in there to clean
it up. At the moment, the underlying RecordWrite() overwrites some input
registers, for example.
That makes sense. I'd forgotten that the stub is actually the rare case, so the
impact of the push and pop is minimal anyway.

[rmcilroy, 2014/02/14 13:44:18]

Ok, done.  Just FYI - I expect we will be adding more of these "short
non-call-marked" stubs in the future since we are planning on doing some work to
avoid inlining exceptional cases (such as this) and move that work into stubs
instead to reduce code-bloat.

+
+  if (!skip_fastpath()) {
+    // Try to convert with a FPU convert instruction.  This handles all
+    // non-saturating cases.
+    __ Push(double_scratch);

[jbramley, 2014/02/13 14:20:14]

If you could combine this with the previous Push, that would be nice. There is a
small overhead to every Push call.

[rmcilroy, 2014/02/14 12:45:07]

I can't - Push() barfs that the registers are of different types (Register and
DoubleRegister).

[jbramley, 2014/02/14 12:57:38]

Ah, yes, sorry.

+    if (input.is(jssp)) double_offset += 1 * kDoubleSize;
+    __ Ldr(double_scratch, MemOperand(input, double_offset));
+    __ TryInlineTruncateDoubleToI(result, double_scratch, &done);
+    __ Fmov(result, double_scratch);
+  } else {
+    __ Ldr(result, MemOperand(input, double_offset));
+  }
+
+  // If we reach here we need to manually convert the input to an int32.
+
+  // Extract the exponent.
+  Register exponent = scratch1;
+  __ Ubfx(exponent, result, HeapNumber::kMantissaBits,
+          HeapNumber::kExponentBits);
+
+  // It the exponent is >= 84 (kMantissaBits + 32), the result is always 0 since
+  // the mantissa gets shifted completely out of the int32_t result.
+  __ Cmp(exponent, HeapNumber::kExponentBias + HeapNumber::kMantissaBits + 32);
+  __ CzeroX(result, ge);
+  __ B(ge, &done);
+
+  // The Fcvtzs sequence handles all cases except where the conversion causes
+  // signed overflow in the int64_t target. Since we've already handled
+  // exponents >= 84, we can guarantee that 63 <= exponent < 84.
+
+  if (masm->emit_debug_code()) {
+    __ Cmp(exponent, HeapNumber::kExponentBias + 63);
+    // Exponents less than this should have been handled by the Fcvt case.
+    __ Check(ge, kUnexpectedValue);
+  }
+
+  // Isolate the mantissa bits, and set the implicit '1'.
+  Register mantissa = scratch2;
+  __ Ubfx(mantissa, result, 0, HeapNumber::kMantissaBits);
+  __ Orr(mantissa, mantissa, 1UL << HeapNumber::kMantissaBits);
+
+  // Negate the mantissa if necessary.
+  __ Tst(result, kXSignMask);
+  __ Cneg(mantissa, mantissa, ne);
+
+  // Shift the mantissa bits in the correct place. We know that we have to shift
+  // it left here, because exponent >= 63 >= kMantissaBits.
+  __ Sub(exponent, exponent,
+         HeapNumber::kExponentBias + HeapNumber::kMantissaBits);
+  __ Lsl(result, mantissa, exponent);
+
+  __ Bind(&done);
+  if (!skip_fastpath()) {
+    __ Pop(double_scratch);
+  }
+  __ Pop(scratch2, scratch1);
+  __ Ret();
+}
+
+
 // See call site for description.
 static void EmitIdenticalObjectComparison(MacroAssembler* masm,
                                           Register left,
                                           Register right,
                                           Register scratch,
                                           FPRegister double_scratch,
                                           Label* slow,
                                           Condition cond) {
   ASSERT(!AreAliased(left, right, scratch));
   Label not_identical, return_equal, heap_number;
@@ skipping 5163 matching lines @@
                               MemOperand(fp, 6 * kPointerSize),
                               NULL);
 }
 
 
 #undef __
 
 } }  // namespace v8::internal
 
 #endif  // V8_TARGET_ARCH_A64