Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(1675)

Issue 2470001: Change keyed load IC interface on x64 to pass arguments in registers. (Closed)

Created:
10 years, 6 months ago by William Hesse
Modified:
9 years, 7 months ago
CC:
v8-dev
Visibility:
Public.

Description

Change keyed load IC interface on x64 to pass arguments in registers. Committed: http://code.google.com/p/v8/source/detail?r=4787

Patch Set 1 #

Patch Set 2 : '' #

Patch Set 3 : '' #

Total comments: 10

Patch Set 4 : '' #

Patch Set 5 : '' #

Unified diffs Side-by-side diffs Delta from patch set Stats (+199 lines, -195 lines) Patch
M src/ia32/codegen-ia32.cc View 1 2 3 4 1 chunk +3 lines, -0 lines 0 comments Download
M src/ia32/ic-ia32.cc View 1 2 3 4 2 chunks +2 lines, -3 lines 0 comments Download
M src/x64/builtins-x64.cc View 1 2 3 4 2 chunks +2 lines, -5 lines 0 comments Download
M src/x64/codegen-x64.cc View 1 2 3 4 9 chunks +34 lines, -20 lines 0 comments Download
M src/x64/debug-x64.cc View 1 2 3 4 1 chunk +3 lines, -2 lines 0 comments Download
M src/x64/full-codegen-x64.cc View 1 2 3 4 7 chunks +11 lines, -11 lines 0 comments Download
M src/x64/ic-x64.cc View 1 2 3 4 13 chunks +115 lines, -114 lines 0 comments Download
M src/x64/stub-cache-x64.cc View 1 2 3 4 9 chunks +24 lines, -38 lines 0 comments Download
M src/x64/virtual-frame-x64.cc View 1 2 3 4 2 chunks +5 lines, -2 lines 0 comments Download

Messages

Total messages: 3 (0 generated)
William Hesse
The last one!
10 years, 6 months ago (2010-06-02 12:21:56 UTC) #1
Søren Thygesen Gjesse
LGTM http://codereview.chromium.org/2470001/diff/13/22004 File src/x64/codegen-x64.cc (right): http://codereview.chromium.org/2470001/diff/13/22004#newcode7599 src/x64/codegen-x64.cc:7599: if (persist_after_get_) { On ARM Dup2 duplicated the ...
10 years, 6 months ago (2010-06-02 13:14:30 UTC) #2
William Hesse
10 years, 6 months ago (2010-06-02 14:37:31 UTC) #3
Comments addressed.

http://codereview.chromium.org/2470001/diff/13/22004
File src/x64/codegen-x64.cc (right):

http://codereview.chromium.org/2470001/diff/13/22004#newcode7479
src/x64/codegen-x64.cc:7479: // from the root away to load null_value, since the
load must be patched
root array, not woot away.

http://codereview.chromium.org/2470001/diff/13/22004#newcode7599
src/x64/codegen-x64.cc:7599: if (persist_after_get_) {
On 2010/06/02 13:14:30, Søren Gjesse wrote:
> On ARM Dup2 duplicated the top two elements on the stack.
> 
> You should be able to use
> 
>   cgen_->frame()->PushElementAt(1);
> 
> (twice) here instead.

Done.

http://codereview.chromium.org/2470001/diff/13/22006
File src/x64/full-codegen-x64.cc (right):

http://codereview.chromium.org/2470001/diff/13/22006#newcode1836
src/x64/full-codegen-x64.cc:1836: __ pop(rbx);
On 2010/06/02 13:14:30, Søren Gjesse wrote:
> ... and remove this pop. I don't see this popped receiver being used.

It is used in line 1844.

http://codereview.chromium.org/2470001/diff/13/22007
File src/x64/ic-x64.cc (right):

http://codereview.chromium.org/2470001/diff/13/22007#newcode358
src/x64/ic-x64.cc:358: // Load name and receiver.
On 2010/06/02 13:14:30, Søren Gjesse wrote:
> Code in comments.

Done.

http://codereview.chromium.org/2470001/diff/13/22010
File src/x64/virtual-frame-x64.h (right):

http://codereview.chromium.org/2470001/diff/13/22010#newcode390
src/x64/virtual-frame-x64.h:390: void Dup2() { PushFrameSlotAt(element_count() -
2); }
On 2010/06/02 13:14:30, Søren Gjesse wrote:
> In the ARM implementation Dup2 duplicates the top two elements on the stack.
> 
> This is equivalent to PushElementAt(1);

Removed.
>

Powered by Google App Engine
This is Rietveld 408576698