Issue 10795: Fixes to RegExp-IA32.

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 10795: Fixes to RegExp-IA32. (Closed)

Created:
12 years ago by Lasse Reichstein

Modified:
9 years, 6 months ago

Reviewers:
Erik Corry

CC:
v8-dev

Visibility:
Public.

Description

Made char comparisons work Tracer for regexp macro-assembler instructions

Patch Set 1 #

Created: 12 years ago

Download [raw] [tar.bz2]

	Unified diffs	Side-by-side diffs	Stats (+452 lines, -15 lines)			Patch
M	src/SConscript	View	1 chunk	+3 lines, -1 line	0 comments	Download
M	src/assembler-ia32.h	View	1 chunk	+1 line, -1 line	0 comments	Download
M	src/assembler-ia32.cc	View	1 chunk	+1 line, -1 line	0 comments	Download
M	src/flag-definitions.h	View	1 chunk	+4 lines, -0 lines	1 comment	Download
M	src/jsregexp.cc	View	3 chunks	+17 lines, -6 lines	0 comments	Download
M	src/regexp-macro-assembler-ia32.cc	View	2 chunks	+8 lines, -6 lines	2 comments	Download
A	src/regexp-macro-assembler-tracer.h	View	1 chunk	+131 lines, -0 lines	1 comment	Download
A	src/regexp-macro-assembler-tracer.cc	View	1 chunk	+287 lines, -0 lines	0 comments	Download

Messages

Total messages: 2 (0 generated)

Expand Messages | Collapse Messages

Erik Corry

12 years ago (2008-11-27 13:04:35 UTC) #2

Just needs a new test then it will LGTM!

http://codereview.chromium.org/10795/diff/1/5
File src/flag-definitions.h (right):

http://codereview.chromium.org/10795/diff/1/5#newcode300
Line 300: DEFINE_bool(trace_regexp_assembler, false,
presumably this only exists in debug mode.
While you are at it can you move trace_regexp_bytecodes here too so it isn't
available on release mode (where it has no effect)?

http://codereview.chromium.org/10795/diff/1/7
File src/regexp-macro-assembler-ia32.cc (right):

http://codereview.chromium.org/10795/diff/1/7#newcode198
Line 198: __ rep_cmpsl();
Premature optimization!
Did you get rid of ASCII mode?

http://codereview.chromium.org/10795/diff/1/7#newcode234
Line 234: __ rep_cmpsb();
One of the if bodies is a suffix of the other.  That suggests a simplification!

http://codereview.chromium.org/10795/diff/1/9
File src/regexp-macro-assembler-tracer.h (right):

http://codereview.chromium.org/10795/diff/1/9#newcode44
Line 44: // character must be from start to start + length_of_bitmap_in_bits.
I think the indivdual method comments should be removed here.  They are just
copies of those from the abstract base class and they are bound to become out of
date.

Expand Messages | Collapse Messages