Issue 149673: Disable inline caching on X64 separately for loads, stores, and calls, while ...

Issue 149673: Disable inline caching on X64 separately for loads, stores, and calls, while ... (Closed)

Created:
11 years, 5 months ago by William Hesse

Modified:
9 years, 7 months ago

Reviewers:
Kasper Lund

CC:
v8-dev

Base URL:
http://v8.googlecode.com/svn/branches/bleeding_edge/

Visibility:
Public.

Description

Disable inline caching on X64 separately for loads, stores, and calls, while they are being implemented and tested one-by-one. Committed: http://code.google.com/p/v8/source/detail?r=2477

Patch Set 1 #

Total comments: 8

Patch Set 2 : '' #

Created: 11 years, 5 months ago

Download [raw] [tar.bz2]

	Unified diffs	Side-by-side diffs	Delta from patch set	Stats (+83 lines, -91 lines)			Patch
M	src/bootstrapper.cc	View	1	1 chunk	+0 lines, -6 lines	0 comments	Download
M	src/ia32/ic-ia32.cc	View	1	1 chunk	+3 lines, -0 lines	0 comments	Download
M	src/ic.h	View	1	2 chunks	+4 lines, -3 lines	0 comments	Download
M	src/ic.cc	View	1	10 chunks	+24 lines, -4 lines	0 comments	Download
M	src/x64/assembler-x64.cc	View	1	1 chunk	+0 lines, -45 lines	0 comments	Download
M	src/x64/debug-x64.cc	View	1	1 chunk	+15 lines, -0 lines	0 comments	Download
M	src/x64/ic-x64.cc	View	1	6 chunks	+37 lines, -33 lines	0 comments	Download

Messages

Total messages: 3 (0 generated)

Expand Messages | Collapse Messages

Kasper Lund

LGTM if you fix line 747. http://codereview.chromium.org/149673/diff/1/3 File src/bootstrapper.cc (left): http://codereview.chromium.org/149673/diff/1/3#oldcode1137 Line 1137: I think ...

11 years, 5 months ago (2009-07-15 11:52:15 UTC) #2

William Hesse

11 years, 5 months ago (2009-07-15 12:26:48 UTC) #3

http://codereview.chromium.org/149673/diff/1/3
File src/bootstrapper.cc (left):

http://codereview.chromium.org/149673/diff/1/3#oldcode1137
Line 1137: 
On 2009/07/15 11:52:15, Kasper Lund wrote:
> I think the empty line was here for a reason. There are other statement blocks
> with empty lines at the bottom if they are followed by an else part in this
> file. I would leave it in.

Done.

http://codereview.chromium.org/149673/diff/1/4
File src/ic.cc (right):

http://codereview.chromium.org/149673/diff/1/4#newcode747
Line 747: if (false && FLAG_use_ic) {
On 2009/07/15 11:52:15, Kasper Lund wrote:
> Please don't commit this.

Changed to an #ifndef block.

http://codereview.chromium.org/149673/diff/1/4#newcode989
Line 989: // TODO(X64): Enable inline cache for StoreIC.
On 2009/07/15 11:52:15, Kasper Lund wrote:
> Why not wrap the entire if (FLAG_use_ic) like the other places?

We needed to use LookupForWrite.  Made explicit.

http://codereview.chromium.org/149673/diff/1/4#newcode989
Line 989: // TODO(X64): Enable inline cache for StoreIC.
On 2009/07/15 11:52:15, Kasper Lund wrote:
> Weird indentation.

Done.

Expand Messages | Collapse Messages