Issue 12469: * Better factoring of ARM/IA32 code in irregexp.

Issue 12469: * Better factoring of ARM/IA32 code in irregexp. (Closed)

Created:
12 years ago by Lasse Reichstein

Modified:
9 years, 6 months ago

Reviewers:
iposva, Kevin Millikin (Chromium), Erik Corry

CC:
v8-dev

Visibility:
Public.

Description

Made ARM/IA32 handling in Regexp symmetric (although without an ARM implementation yet).

Patch Set 1 #

Patch Set 2 : Also added ..-arm.cc file #

Patch Set 3 : Dropped the unifying ...-native.h file. #

Created: 12 years ago

Download [raw] [tar.bz2]

	Unified diffs	Side-by-side diffs	Delta from patch set	Stats (+150 lines, -56 lines)			Patch
M	src/SConscript	View		1 chunk	+2 lines, -1 line	0 comments	Download
M	src/assembler.h	View		1 chunk	+2 lines, -1 line	0 comments	Download
M	src/ast.h	View		1 chunk	+1 line, -4 lines	0 comments	Download
M	src/ast.cc	View		1 chunk	+1 line, -1 line	0 comments	Download
M	src/codegen.h	View		1 chunk	+1 line, -1 line	0 comments	Download
M	src/execution.cc	View	1 2	1 chunk	+1 line, -1 line	0 comments	Download
M	src/frames-inl.h	View	1 2	1 chunk	+1 line, -1 line	0 comments	Download
M	src/jsregexp.cc	View	1 2	7 chunks	+22 lines, -19 lines	0 comments	Download
M	src/jsregexp-inl.h	View		2 chunks	+4 lines, -2 lines	0 comments	Download
M	src/macro-assembler.h	View		1 chunk	+1 line, -1 line	0 comments	Download
A	src/regexp-macro-assembler-arm.h	View	1 2	1 chunk	+41 lines, -0 lines	0 comments	Download
A	src/regexp-macro-assembler-arm.cc	View	2	1 chunk	+43 lines, -0 lines	0 comments	Download
M	src/regexp-macro-assembler-ia32.h	View	1 2	6 chunks	+16 lines, -18 lines	0 comments	Download
M	src/regexp-macro-assembler-ia32.cc	View	1 2	5 chunks	+7 lines, -4 lines	0 comments	Download
M	test/cctest/test-regexp.cc	View	1 2	3 chunks	+7 lines, -2 lines	0 comments	Download

Messages

Total messages: 4 (0 generated)

Expand Messages | Collapse Messages

Lasse Reichstein

Restructured ARM/IA32 division in new regexp code (+ minor readability fixups).

12 years ago (2008-11-26 09:40:18 UTC) #1

Kevin Millikin (Chromium)

http://codereview.chromium.org/12469/diff/22/202 File src/execution.cc (right): http://codereview.chromium.org/12469/diff/22/202#newcode35 Line 35: #if defined(ARM) #ifdef ARM

12 years ago (2008-11-26 10:05:59 UTC) #2

Erik Corry

I'm afraid I don't like parts of this change. The new RegExpMacroAssemblerImpl typedef is only ...

12 years ago (2008-11-26 12:48:05 UTC) #3

Lasse Reichstein

12 years ago (2008-11-26 13:57:39 UTC) #4

No extra include file any more. A few more "#if defined(ARM)" fixed.

http://codereview.chromium.org/12469/diff/22/202
File src/execution.cc (right):

http://codereview.chromium.org/12469/diff/22/202#newcode35
Line 35: #if defined(ARM)
On 2008/11/26 10:05:59, Kevin Millikin wrote:
> #ifdef ARM

Good catch!

http://codereview.chromium.org/12469/diff/22/204
File src/jsregexp.cc (right):

http://codereview.chromium.org/12469/diff/22/204#newcode46
Line 46: #include "regexp-macro-assembler-native.h"
It's gone.
It was used in only two places (jsregexp.cc and test-regexp.cc). The includes
from the native.h-file are now inlined in these two places.

http://codereview.chromium.org/12469/diff/22/206
File src/regexp-macro-assembler-arm.cc (right):

http://codereview.chromium.org/12469/diff/22/206#newcode39
Line 39: : RegExpMacroAssemblerIrregexp(NULL) {}
I think UNIMPLEMENTED is better. UNREACHABLE is a debug-only check.

http://codereview.chromium.org/12469/diff/22/210
File src/regexp-macro-assembler-native.h (right):

http://codereview.chromium.org/12469/diff/22/210#newcode39
Line 39: typedef RegExpMacroAssemblerARM RegExpMacroAssemblerImpl;
It's gone.

Expand Messages | Collapse Messages