https://codereview.chromium.org/160423002/diff/160001/src/a64/assembler-a64.cc
File src/a64/assembler-a64.cc (right):
https://codereview.chromium.org/160423002/diff/160001/src/a64/assembler-a64.cc#newcode183
src/a64/assembler-a64.cc:183: Register
GetRegisterThatIsNotOneOf(Register reg1, Register reg2,
I'd call it GetAllocatableRegisterThatIsNotOneOf(...).
https://codereview.chromium.org/160423002/diff/160001/src/a64/assembler-a64.cc#newcode192
src/a64/assembler-a64.cc:192: if (reg6.IsValid()) regs |= reg6.Bit();
CPURegList can do a lot of this for you:
CPURegList regs(reg1, reg2, ...);
for (...) {
if (regs.IncludesAliasOf(candidate)) continue;
}
https://codereview.chromium.org/160423002/diff/160001/src/a64/code-stubs-a64.cc
File src/a64/code-stubs-a64.cc (right):
https://codereview.chromium.org/160423002/diff/160001/src/a64/code-stubs-a64.cc#newcode539
src/a64/code-stubs-a64.cc:539: void
DoubleToIStub::Generate(MacroAssembler* masm) {
It seems that this stub doesn't always truncate on other architectures.
We should ASSERT(is_truncating()) somewhere.
https://codereview.chromium.org/160423002/diff/160001/src/a64/code-stubs-a64.cc#newcode549
src/a64/code-stubs-a64.cc:549: if (input.is(jssp)) double_offset += 2 *
kPointerSize;
Do this near Push(scratch1, scratch2) so it's clear what it's for.
https://codereview.chromium.org/160423002/diff/160001/src/a64/code-stubs-a64.cc#newcode551
src/a64/code-stubs-a64.cc:551: DoubleRegister double_scratch = d0; //
only used if !skip_fastpath()
In that case, put it in the !skip_fastpath() scope.
https://codereview.chromium.org/160423002/diff/160001/src/a64/code-stubs-a64.cc#newcode556
src/a64/code-stubs-a64.cc:556: __ Push(scratch1, scratch2);
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.
https://codereview.chromium.org/160423002/diff/160001/src/a64/code-stubs-a64.cc#newcode561
src/a64/code-stubs-a64.cc:561: __ Push(double_scratch);
If you could combine this with the previous Push, that would be nice.
There is a small overhead to every Push call.
https://codereview.chromium.org/160423002/diff/160001/src/a64/macro-assembler-a64.cc
File src/a64/macro-assembler-a64.cc (right):
https://codereview.chromium.org/160423002/diff/160001/src/a64/macro-assembler-a64.cc#newcode2515
src/a64/macro-assembler-a64.cc:2515: DoubleToIStub stub(jssp, result, 0,
true, true);
Please use comments to label those bools (or make them enums and update
the other back-ends accordingly).
https://codereview.chromium.org/160423002/diff/160001/src/a64/macro-assembler-a64.cc#newcode2525
src/a64/macro-assembler-a64.cc:2525: Sxtw(result, result);
Use Sxtw(result, result.W());
This enables a more strict assertion inside the assembler. Really, we
shouldn't allow Sxtw(xd, xn) because it doesn't make sense, but that's
an issue we're thinking about anyway.
https://codereview.chromium.org/160423002/diff/160001/src/a64/macro-assembler-a64.cc#newcode2548
src/a64/macro-assembler-a64.cc:2548: bind(&done);
Bind(&done);
https://codereview.chromium.org/160423002/diff/160001/test/cctest/test-code-stubs-a64.cc
File test/cctest/test-code-stubs-a64.cc (right):
https://codereview.chromium.org/160423002/diff/160001/test/cctest/test-code-stubs-a64.cc#newcode100
test/cctest/test-code-stubs-a64.cc:100: // // Make sure no registers
have been unexpectedly clobbered
Double comment.
https://codereview.chromium.org/160423002/diff/160001/test/cctest/test-code-stubs-a64.cc#newcode113
test/cctest/test-code-stubs-a64.cc:113: __ Mov(x0, destination_reg);
The 'if' isn't necessary; A64's Mov emits no code if the source and
destination are the same.
https://codereview.chromium.org/160423002/diff/160001/test/cctest/test-code-stubs-a64.cc#newcode137
test/cctest/test-code-stubs-a64.cc:137: int32_t
RunGeneratedCodeCallWrapper(ConvertDToIFunc func,
Does this need to return int64_t? If not, the result of CallInt64 should
probably be filtered through is_int32 or something similar.
https://codereview.chromium.org/160423002/
--
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
---
You received this message because you are subscribed to the Google Groups "v8-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
For more options, visit https://groups.google.com/groups/opt_out.
