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

Issue 9179010: Fix and enable NEW_NON_STRICT_FAST ArgumentsAccess stub on x64. (Closed)

Created:
8 years, 11 months ago by Vyacheslav Egorov (Chromium)
Modified:
8 years, 11 months ago
Reviewers:
fschneider
CC:
v8-dev
Visibility:
Public.

Description

Fix and enable NEW_NON_STRICT_FAST ArgumentsAccess stub on x64. R=fschneider@chromium.org BUG=v8:1903 Committed: http://code.google.com/p/v8/source/detail?r=10411

Patch Set 1 #

Total comments: 4
Unified diffs Side-by-side diffs Delta from patch set Stats (+14 lines, -11 lines) Patch
M src/x64/code-stubs-x64.cc View 3 chunks +4 lines, -7 lines 3 comments Download
M src/x64/full-codegen-x64.cc View 1 chunk +10 lines, -4 lines 1 comment Download

Messages

Total messages: 3 (0 generated)
Vyacheslav Egorov (Chromium)
8 years, 11 months ago (2012-01-16 13:25:27 UTC) #1
fschneider
LGTM. http://codereview.chromium.org/9179010/diff/1/src/x64/code-stubs-x64.cc File src/x64/code-stubs-x64.cc (right): http://codereview.chromium.org/9179010/diff/1/src/x64/code-stubs-x64.cc#newcode2314 src/x64/code-stubs-x64.cc:2314: // rsp[16] : receiver displacement The comment is ...
8 years, 11 months ago (2012-01-16 14:25:07 UTC) #2
fschneider
8 years, 11 months ago (2012-01-16 14:26:18 UTC) #3
Please make sure that we have test coverage of the different cases in the stub.

On 2012/01/16 14:25:07, fschneider wrote:
> LGTM.
> 
> http://codereview.chromium.org/9179010/diff/1/src/x64/code-stubs-x64.cc
> File src/x64/code-stubs-x64.cc (right):
> 
>
http://codereview.chromium.org/9179010/diff/1/src/x64/code-stubs-x64.cc#newco...
> src/x64/code-stubs-x64.cc:2314: //  rsp[16] : receiver displacement
> The comment is misleading: This argument is an absolute address, not a
> displacement.
> 
>
http://codereview.chromium.org/9179010/diff/1/src/x64/code-stubs-x64.cc#newco...
> src/x64/code-stubs-x64.cc:2454: __ Integer64PlusConstantToSmi(r9, rbx, 0);
> Isn't there an instruction to make a smi without adding a constant?
> 
>
http://codereview.chromium.org/9179010/diff/1/src/x64/code-stubs-x64.cc#newco...
> src/x64/code-stubs-x64.cc:2496: // Untag rcx and r8 for the loop below.
> Comment should say: Untag rcx for the loop below.
> 
> (r8 already untagged)
> 
> http://codereview.chromium.org/9179010/diff/1/src/x64/full-codegen-x64.cc
> File src/x64/full-codegen-x64.cc (right):
> 
>
http://codereview.chromium.org/9179010/diff/1/src/x64/full-codegen-x64.cc#new...
> src/x64/full-codegen-x64.cc:225: // Arguments to ArgumentsAccessStub and/or
> New...:
> What's "New..."? I think the comment was fine as it was.

Powered by Google App Engine
This is Rietveld 408576698