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

Issue 16512: Experimental: begin using the register allocator across entering and... (Closed)

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

Description

Experimental: begin using the register allocator across entering and leaving with statements. This requires more care with the use of the context (esi on IA32) register and the frame slot allocated for saving it. Committed: http://code.google.com/p/v8/source/detail?r=1027

Patch Set 1 #

Patch Set 2 : '' #

Total comments: 4
Unified diffs Side-by-side diffs Delta from patch set Stats (+73 lines, -29 lines) Patch
M src/codegen-ia32.cc View 1 8 chunks +20 lines, -18 lines 2 comments Download
M src/virtual-frame-ia32.h View 1 2 chunks +7 lines, -4 lines 2 comments Download
M src/virtual-frame-ia32.cc View 1 3 chunks +46 lines, -7 lines 0 comments Download

Messages

Total messages: 3 (0 generated)
Kevin Millikin (Chromium)
11 years, 11 months ago (2009-01-06 10:48:37 UTC) #1
William Hesse
LGTM, with comments. http://codereview.chromium.org/16512/diff/6/207 File src/codegen-ia32.cc (right): http://codereview.chromium.org/16512/diff/6/207#newcode1677 Line 1677: __ cmp(context.reg(), Operand(esi)); Can't context.Unuse() ...
11 years, 11 months ago (2009-01-06 12:04:07 UTC) #2
Kevin Millikin (Chromium)
11 years, 11 months ago (2009-01-06 12:10:28 UTC) #3
http://codereview.chromium.org/16512/diff/6/207
File src/codegen-ia32.cc (right):

http://codereview.chromium.org/16512/diff/6/207#newcode1677
Line 1677: __ cmp(context.reg(), Operand(esi));
On 2009/01/06 12:04:08, William Hesse wrote:
> Can't context.Unuse() go directly after the cmp instruction?

Yes.  Fixed.

http://codereview.chromium.org/16512/diff/6/206
File src/virtual-frame-ia32.h (right):

http://codereview.chromium.org/16512/diff/6/206#newcode284
Line 284: // Save the value of the esi register in the context frame slot.
On 2009/01/06 12:04:08, William Hesse wrote:
> Comment should say that that esi is not synced to memory until spilled.
> // Save the esi register to the virtual context frame slot.
> // The value is not written to memory until synced.
> Should the code optimize the case where the context slot is already esi?

Comment changed.

Powered by Google App Engine
This is Rietveld 408576698