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

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

Created:
10 years, 10 months ago by William Hesse
Modified:
9 years, 7 months ago
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 Delta from patch set Stats (+3 lines, -0 lines) Patch
M src/ia32/codegen-ia32.cc View 1 1 chunk +3 lines, -0 lines 1 comment Download

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
This is Rietveld 408576698