Chromium Code Reviews

Issue 651019: Improve load of global variables. (Closed)

Created:
10 years, 10 months ago by William Hesse
Modified:
9 years, 7 months ago
Reviewers:
Kevin Millikin (Chromium), Mads Ager (chromium)
CC:
v8-dev
Visibility:
Public.

Description

Improve load of global variables. Committed: http://code.google.com/p/v8/source/detail?r=3911

Patch Set 1 #

Patch Set 2 : '' #

Total comments: 1
Unified diffs Side-by-side diffs Stats (+3 lines, -0 lines)
M src/ia32/codegen-ia32.cc View 1 chunk +3 lines, -0 lines 1 comment

Messages

Total messages: 3 (0 generated)
William Hesse
10 years, 10 months ago (2010-02-19 09:39:19 UTC) #1
Mads Ager (chromium)
LGTM I think the comment could be extended a bit to make it clear why ...
10 years, 10 months ago (2010-02-19 10:01:33 UTC) #2
Kevin Millikin (Chromium)
10 years, 10 months ago (2010-02-19 10:23:04 UTC) #3
Questions:

1. If this is a good idea, why not do it everywhere
(LoadFromGlobalSlotCheckExtensions, EmitNamedPropertyAssignment for compound
assignments)?

2. Would it be easy to port this change to x64?

http://codereview.chromium.org/651019/diff/1002/6
File src/ia32/codegen-ia32.cc (right):

http://codereview.chromium.org/651019/diff/1002/6#newcode693
src/ia32/codegen-ia32.cc:693: // Spilling eax makes it free, so LoadGlobal loads
directly into eax.
The comment is still not quite right.  LoadIC wants receiver in eax and the
register allocator will prefer eax if free.

I think it's obvious that Spill(eax) makes it free, but not that the register
allocator prefers it.

Named stores do use reference in the case of e.x++ and ++e.x, so don't put that
comment at all since it's not true.

Powered by Google App Engine