Issue 598016: Restrict the syntax that we aggressively optimize.

Keyboard Shortcuts

	File
u :	up to issue
j / k :	jump to file after / before current file
J / K :	jump to next file with a comment after / before current file
	Side-by-side diff
i :	toggle intra-line diffs
e :	expand all comments
c :	collapse all comments
s :	toggle showing all comments
n / p :	next / previous diff chunk or comment
N / P :	next / previous comment
<Up> / <Down> :	next / previous line

	Issue
u :	up to list of issues
j / k :	jump to patch after / before current patch
o / <Enter> :	open current patch in side-by-side view
i :	open current patch in unified diff view

	Issue List
j / k :	jump to issue after / before current issue
o / <Enter> :	open current issue

Issue 598016: Restrict the syntax that we aggressively optimize. (Closed)

Created:
10 years, 10 months ago by Kevin Millikin (Chromium)

Modified:
9 years, 7 months ago

Reviewers:
fschneider

CC:
v8-dev

Visibility:
Public.

Description

Restrict the syntax that we aggressively optimize. Do not use the speculative compiler for functions with other than one statement in the body, and do not use it if subexpressions can have side effects. Bailing out to the beginning of the full code is not sound if side effects have already occurred. Add tests that would fail without the restrictions. Committed: http://code.google.com/p/v8/source/detail?r=3826

Patch Set 1 #

Total comments: 2

Created: 10 years, 10 months ago

Download [raw] [tar.bz2]

		Unified diffs	Side-by-side diffs	Delta from patch set	Stats (+146 lines, -3 lines)			Patch
	M	src/fast-codegen.cc	View		4 chunks	+19 lines, -3 lines	0 comments	Download
	A	test/mjsunit/compiler/simple-bailouts.js	View		1 chunk	+127 lines, -0 lines	2 comments	Download

Messages

Total messages: 2 (0 generated)

Expand Messages | Collapse Messages

fschneider

10 years, 10 months ago (2010-02-09 16:59:10 UTC) #2

LGTM

http://codereview.chromium.org/598016/diff/1/3
File test/mjsunit/compiler/simple-bailouts.js (right):

http://codereview.chromium.org/598016/diff/1/3#newcode86
test/mjsunit/compiler/simple-bailouts.js:86: this.z = // (14 | 1.1) == 15
I'd align the // vertically in this function as well.

http://codereview.chromium.org/598016/diff/1/3#newcode92
test/mjsunit/compiler/simple-bailouts.js:92: | a;  // 1.0
// 1.1

Expand Messages | Collapse Messages