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

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

Can't Edit
Can't Publish+Mail
Start Review
Created:
3 years, 4 months ago by Kevin Millikin
Modified:
2 years, 11 months ago
Reviewers:
fschneider
CC:
v8-dev_googlegroups.com
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) Lint Patch
M src/ast.h View 5 chunks +13 lines, -3 lines 0 comments 0 errors Download
M src/ast.cc View 1 chunk +6 lines, -5 lines 0 comments 0 errors Download
M src/ast-inl.h View 1 chunk +2 lines, -1 line 0 comments 0 errors Download
M src/full-codegen.h View 1 chunk +1 line, -1 line 0 comments 0 errors Download
M src/hydrogen.cc View 11 chunks +51 lines, -50 lines 0 comments 0 errors Download
M src/ia32/full-codegen-ia32.cc View 12 chunks +19 lines, -8 lines 0 comments 0 errors Download
A test/mjsunit/regress/regress-989.js View 1 chunk +127 lines, -0 lines 0 comments ? errors Download
Trybot results:
Commit:

Messages

Total messages: 2
Kevin Millikin
3 years, 4 months ago #1
fschneider
3 years, 4 months ago #2
LGTM.
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld 1275:d14800f88434