http://codereview.chromium.org/2111011/diff/10001/11002
File src/x64/codegen-x64.cc (right):
http://codereview.chromium.org/2111011/diff/10001/11002#newcode694
src/x64/codegen-x64.cc:694: } else if (receiver_.is(value_)) {
On 2010/05/20 11:54:32, Søren Gjesse wrote:
I don't get this, can receiver_.is(value_) ever be true?
Yes. var x=new Object(); x[i] = x;
http://codereview.chromium.org/2111011/diff/10001/11002#newcode704
src/x64/codegen-x64.cc:704: // Value is now in rax. Its original
location is remembered in value_.
On 2010/05/20 11:54:32, Søren Gjesse wrote:
Maybe extend comment to say that value_ is used later to store the
result, and
that receiver_ and key_ might not have their original values.
Alternatively use two locals receiver and key to avoid overwriting
receiver_ and
key_ when swapping with rax.
Done.
http://codereview.chromium.org/2111011/diff/10001/11002#newcode718
src/x64/codegen-x64.cc:718: }
If key_ is rdx and receiver_ is rbx, we need to move key_ first.
On 2010/05/20 11:54:32, Søren Gjesse wrote:
How about
} else
// receiver_ is neither rcx nor rdx.
__ movq(rdx, receiver_);
if (!key_.is(rcx)) {
__ movq(rcx, key_);
}
}
instead of
} else if (key_.is(rcx)) {
__ movq(rdx, receiver_);
} else {
__ movq(rcx, key_);
__ movq(rdx, receiver_);
}
http://codereview.chromium.org/2111011/diff/10001/11005
File src/x64/ic-x64.cc (right):
http://codereview.chromium.org/2111011/diff/10001/11005#newcode900
src/x64/ic-x64.cc:900: // rdx: JSArray
On 2010/05/20 11:54:32, Søren Gjesse wrote:
JSArray -> receiver (a JSArray)
Like in the comment below.
Done.
http://codereview.chromium.org/2111011/diff/10001/11005#newcode1065
src/x64/ic-x64.cc:1065: __ pop(rdx);
On 2010/05/20 11:54:32, Søren Gjesse wrote:
Please add the comment on rbx and rdi content here as well.
Done.
http://codereview.chromium.org/2111011/diff/10001/11005#newcode1102
src/x64/ic-x64.cc:1102: __ bind(&is_nan);
On 2010/05/20 11:54:32, Søren Gjesse wrote:
Please add comment on register content as above.
Done.
http://codereview.chromium.org/2111011/diff/10001/11007
File src/x64/virtual-frame-x64.cc (right):
http://codereview.chromium.org/2111011/diff/10001/11007#newcode1035
src/x64/virtual-frame-x64.cc:1035: // are a and b. Other results can be
live, but must not be in a_reg or b_reg.
On 2010/05/20 11:54:32, Søren Gjesse wrote:
Please assert these asumptions.
Done.
http://codereview.chromium.org/2111011/diff/10001/11007#newcode1046
src/x64/virtual-frame-x64.cc:1046: // a must be in b_reg, b in a_reg.
This is only used in setting up arguments in registers for
an ic call, which relies on a spilled frame and does not preserve
registers on return, so we don't need this.
We could also just swap using a temporary variable.
It On 2010/05/20 11:54:32, Søren Gjesse wrote:
How about adding swapping of results to the virtual frame? Then you
don´t need
to rely on a and b being invalidated and the caller can decide when to
invalidate.
http://codereview.chromium.org/2111011/show
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
