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

Issue 150225: Improved code for megamorphic stub on ia32. (Closed)

Created:
11 years, 5 months ago by bak
Modified:
9 years, 7 months ago
Reviewers:
Kasper Lund
CC:
v8-dev
Visibility:
Public.

Description

Improved code for megamorphic stub on ia32. Committed: http://code.google.com/p/v8/source/detail?r=2342

Patch Set 1 #

Total comments: 3
Unified diffs Side-by-side diffs Delta from patch set Stats (+50 lines, -29 lines) Patch
M src/ia32/ic-ia32.cc View 4 chunks +4 lines, -4 lines 0 comments Download
M src/ia32/stub-cache-ia32.cc View 3 chunks +44 lines, -24 lines 1 comment Download
M src/stub-cache.h View 1 chunk +2 lines, -1 line 2 comments Download

Messages

Total messages: 2 (0 generated)
bak
11 years, 5 months ago (2009-07-02 12:34:13 UTC) #1
Kasper Lund
11 years, 5 months ago (2009-07-02 12:53:08 UTC) #2
If you make sure it compiles on non-IA32 platforms, it LGTM...

http://codereview.chromium.org/150225/diff/1/2
File src/ia32/stub-cache-ia32.cc (right):

http://codereview.chromium.org/150225/diff/1/2#newcode52
Line 52: // Get the code entry from the cache.
I take it you've considered having multiple extra.is_valid checks in this
function but decided that made the code too unreadable? It's probably the right
decision...

http://codereview.chromium.org/150225/diff/1/4
File src/stub-cache.h (right):

http://codereview.chromium.org/150225/diff/1/4#newcode199
Line 199: // Generate code for probing the stub cache table.
I think you should add a note about the contract here. Both scratch and extra
will be clobbered, but it's okay to pass in no_reg for extra... or something
like that.

http://codereview.chromium.org/150225/diff/1/4#newcode205
Line 205: Register extra);
Will this change break the compilation of all non-IA32 platforms?

Powered by Google App Engine
This is Rietveld 408576698