Issue 549158: Refactor the selection of code generator (toplevel or optimizing) and...

Issue 549158: Refactor the selection of code generator (toplevel or optimizing) and... (Closed)

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

Modified:
9 years, 7 months ago

Reviewers:
Erik Corry, fschneider

CC:
v8-dev

Base URL:
http://v8.googlecode.com/svn/branches/bleeding_edge/

Visibility:
Public.

Description

Refactor the selection of code generator and add the possibility of a simpler backend for non-toplevel code. Committed: http://code.google.com/p/v8/source/detail?r=3721

Patch Set 5 : '' #

Created: 10 years, 11 months ago

Download [raw] [tar.bz2]

	Unified diffs	Side-by-side diffs	Delta from patch set	Stats (+442 lines, -46 lines)			Patch
M	src/SConscript	View		1 chunk	+1 line, -0 lines	0 comments	Download
M	src/arm/full-codegen-arm.cc	View		1 chunk	+1 line, -1 line	0 comments	Download
M	src/ast.h	View		3 chunks	+4 lines, -4 lines	0 comments	Download
M	src/compiler.cc	View	1 2 3 4	5 chunks	+43 lines, -23 lines	0 comments	Download
A	src/fast-codegen.h	View		1 chunk	+63 lines, -0 lines	0 comments	Download
A	src/fast-codegen.cc	View	2 3 4	1 chunk	+306 lines, -0 lines	0 comments	Download
M	src/flag-definitions.h	View	1 2	1 chunk	+7 lines, -5 lines	0 comments	Download
M	src/full-codegen.h	View		2 chunks	+3 lines, -3 lines	0 comments	Download
M	src/full-codegen.cc	View	1 2	2 chunks	+4 lines, -2 lines	0 comments	Download
M	src/ia32/full-codegen-ia32.cc	View		1 chunk	+2 lines, -2 lines	0 comments	Download
M	src/objects.h	View		2 chunks	+3 lines, -3 lines	0 comments	Download
M	src/objects-inl.h	View		1 chunk	+2 lines, -2 lines	0 comments	Download
M	src/x64/full-codegen-x64.cc	View		1 chunk	+1 line, -1 line	0 comments	Download
M	tools/gyp/v8.gyp	View		1 chunk	+2 lines, -0 lines	0 comments	Download

Messages

Total messages: 4 (0 generated)

Expand Messages | Collapse Messages

fschneider

LGTM (with comments addressed) http://codereview.chromium.org/549158/diff/6001/7002 File src/compiler.cc (right): http://codereview.chromium.org/549158/diff/6001/7002#newcode97 src/compiler.cc:97: ASSERT(!FLAG_always_full_compiler || !FLAG_always_fast_compiler); 'Classic' mode ...

10 years, 11 months ago (2010-01-27 14:30:01 UTC) #2

Erik Corry

I don't like the name 'fast compiler', but I guess it LGTM. http://codereview.chromium.org/549158/diff/6001/7002 File src/compiler.cc ...

10 years, 11 months ago (2010-01-27 14:47:11 UTC) #3

Kevin Millikin (Chromium)

10 years, 11 months ago (2010-01-27 15:09:51 UTC) #4

http://codereview.chromium.org/549158/diff/6001/7002
File src/compiler.cc (right):

http://codereview.chromium.org/549158/diff/6001/7002#newcode97
src/compiler.cc:97: ASSERT(!FLAG_always_full_compiler ||
!FLAG_always_fast_compiler);
On 2010/01/27 14:47:11, Erik Corry wrote:
> I think CHECK would be more appropriate.

OK.

http://codereview.chromium.org/549158/diff/6001/7002#newcode110
src/compiler.cc:110: (FLAG_fast_compiler) && !is_run_once) {
On 2010/01/27 14:30:01, fschneider wrote:
> Misplaced parenthesis? Should it be:
> 
> } else if (FLAG_always_fast_compiler ||
>            (FLAG_fast_compiler && !is_run_once)) {

Yes, thank you.

http://codereview.chromium.org/549158/diff/6001/7008
File src/fast-codegen.cc (right):

http://codereview.chromium.org/549158/diff/6001/7008#newcode78
src/fast-codegen.cc:78: if (decls->length() > 0) {
On 2010/01/27 14:30:01, fschneider wrote:
> VisitDeclarations is not called yet, I'd leave the body emtpy or
UNREACHABLE().
> Or implement it as in the FullCodeGenSyntaxChecker and put in an unconditional
> BAILOUT in VisitDeclaration.
> 

OK.  I originally had it that way so I'll just change it back.

Expand Messages | Collapse Messages