Issue 3327001: ia32 StringAddStub: fast two character case.

Issue 3327001: ia32 StringAddStub: fast two character case. (Closed)

Created:
10 years, 3 months ago by Vitaly Repeshko

Modified:
9 years, 7 months ago

Reviewers:
Søren Thygesen Gjesse

CC:
v8-dev

Visibility:
Public.

Description

ia32 StringAddStub: fast two character case. Committed: http://code.google.com/p/v8/source/detail?r=5397

Patch Set 1 #

Patch Set 2 : Better comment. #

Total comments: 6

Created: 10 years, 3 months ago

Download [raw] [tar.bz2]

	Unified diffs	Side-by-side diffs	Delta from patch set	Stats (+65 lines, -12 lines)			Patch
M	src/ia32/code-stubs-ia32.h	View	1	1 chunk	+8 lines, -4 lines	4 comments	Download
M	src/ia32/code-stubs-ia32.cc	View		5 chunks	+28 lines, -8 lines	0 comments	Download
M	src/ia32/macro-assembler-ia32.h	View		1 chunk	+4 lines, -0 lines	0 comments	Download
M	src/ia32/macro-assembler-ia32.cc	View		1 chunk	+24 lines, -0 lines	2 comments	Download
M	src/v8-counters.h	View		1 chunk	+1 line, -0 lines	0 comments	Download

Messages

Total messages: 3 (0 generated)

Expand Messages | Collapse Messages

Søren Thygesen Gjesse

LGTM Will you port it to x64 and ARM now. If now please open a ...

10 years, 3 months ago (2010-09-01 11:19:26 UTC) #2

Vitaly Repeshko

10 years, 3 months ago (2010-09-01 17:21:51 UTC) #3

Thanks for the review! Submitted.


-- Vitaly

http://codereview.chromium.org/3327001/diff/3001/4002
File src/ia32/code-stubs-ia32.h (right):

http://codereview.chromium.org/3327001/diff/3001/4002#newcode241
src/ia32/code-stubs-ia32.h:241: // performed. This jump does not guarantee that
the string is not in
On 2010/09/01 11:19:31, Søren Gjesse wrote:
> Maybe extend the comment to say that c1 and c2 are clobbered when a jump to
> not_found is performed.

Done.

http://codereview.chromium.org/3327001/diff/3001/4002#newcode250
src/ia32/code-stubs-ia32.h:250: Label* not_probed,
On 2010/09/01 11:19:31, Søren Gjesse wrote:
> How about renaming not_probed to not_found_c1_c2_preserved? Maybe not?

I think having this described in the comment is enough.

http://codereview.chromium.org/3327001/diff/3001/4003
File src/ia32/macro-assembler-ia32.cc (right):

http://codereview.chromium.org/3327001/diff/3001/4003#newcode787
src/ia32/macro-assembler-ia32.cc:787: Register scratch,
On 2010/09/01 11:19:31, Søren Gjesse wrote:
> Maybe add another scratch register and pass that as no_reg, leaving the option
> of avoiding the object reference in the reloc info to the caller.

Done.

Expand Messages | Collapse Messages