Chromium Code Reviews
Help | Chromium Project | Sign in
(14)

Issue 5682010: Deoptimize to the proper target after assignment side effects. (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
4 years, 4 months ago by Kevin Millikin (Chromium)
Modified:
3 years, 12 months ago
Reviewers:
fschneider
CC:
v8-dev
Visibility:
Public.

Description

Deoptimize to the proper target after assignment side effects. This fixes V8 issue 989. Before, assignments used the AST ID of the assignment expression to mark the side effect of the store, which became a target for deoptimization bailout for code after the assignment. In effect contexts this environment included the value of the assignment, which was unexpected by the unoptimized code. Now we introduce a new assignment ID for AST node types that include an assignment (Assignment, CountOperation, and ForInStatement) and use it for the side effect of the store. Committed: http://code.google.com/p/v8/source/detail?r=5990

Patch Set 1 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+219 lines, -68 lines) Patch
M src/ast.h View 5 chunks +13 lines, -3 lines 0 comments Download
M src/ast.cc View 1 chunk +6 lines, -5 lines 0 comments Download
M src/ast-inl.h View 1 chunk +2 lines, -1 line 0 comments Download
M src/full-codegen.h View 1 chunk +1 line, -1 line 0 comments Download
M src/hydrogen.cc View 11 chunks +51 lines, -50 lines 0 comments Download
M src/ia32/full-codegen-ia32.cc View 12 chunks +19 lines, -8 lines 0 comments Download
A test/mjsunit/regress/regress-989.js View 1 chunk +127 lines, -0 lines 0 comments Download
Trybot results:
Commit: CQ not working?

Messages

Total messages: 2 (0 generated)
Kevin Millikin (Chromium)
4 years, 4 months ago (2010-12-13 15:31:38 UTC) #1
fschneider
4 years, 4 months ago (2010-12-13 16:22:59 UTC) #2
LGTM.
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld 98db9e2