Chromium Code Reviews
Help | Chromium Project | Sign in
(10)

Issue 598062: Probe the symbol table for two character strings in native code... (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
5 years, 4 months ago by Søren Thygesen Gjesse
Modified:
4 years, 1 month ago
CC:
v8-dev
Visibility:
Public.

Description

Probe the symbol table for two character strings in native code All two character string results from adding two strings and from sub string used to be handled in the runtime system as a lookup in the symbol table was done before allocating a new string. The native code for string add and sub string now probes the symbol cache for two character strings to avoid the runtime call. If the result string is not found in the symbol table within a fixed number of probes a new string is just allocated. Newly allocated two character strings are not added to the symbol table immediately. Committed: http://code.google.com/p/v8/source/detail?r=3842

Patch Set 1 #

Total comments: 16

Patch Set 2 : '' #

Unified diffs Side-by-side diffs Delta from patch set Stats (+292 lines, -9 lines) Patch
M src/ia32/codegen-ia32.h View 1 1 chunk +25 lines, -0 lines 0 comments Download
M src/ia32/codegen-ia32.cc View 1 4 chunks +246 lines, -9 lines 0 comments Download
M src/ia32/macro-assembler-ia32.h View 1 chunk +7 lines, -0 lines 0 comments Download
M src/ia32/macro-assembler-ia32.cc View 1 chunk +14 lines, -0 lines 0 comments Download
Trybot results:
Commit: CQ not working?

Messages

Total messages: 3 (0 generated)
Søren Thygesen Gjesse
5 years, 4 months ago (2010-02-11 13:21:11 UTC) #1
Mads Ager (chromium)
LGTM http://codereview.chromium.org/598062/diff/1/2 File src/ia32/codegen-ia32.cc (right): http://codereview.chromium.org/598062/diff/1/2#newcode9904 src/ia32/codegen-ia32.cc:9904: Label *not_found) { Label * -> Label* http://codereview.chromium.org/598062/diff/1/2#newcode9910 ...
5 years, 4 months ago (2010-02-12 10:24:05 UTC) #2
Søren Thygesen Gjesse
5 years, 4 months ago (2010-02-12 11:41:32 UTC) #3
http://codereview.chromium.org/598062/diff/1/2
File src/ia32/codegen-ia32.cc (right):

http://codereview.chromium.org/598062/diff/1/2#newcode9904
src/ia32/codegen-ia32.cc:9904: Label *not_found) {
On 2010/02/12 10:24:05, Mads Ager wrote:
> Label * -> Label*
> 

Done.

http://codereview.chromium.org/598062/diff/1/2#newcode9910
src/ia32/codegen-ia32.cc:9910: Label not_aray_index;
On 2010/02/12 10:24:05, Mads Ager wrote:
> not_aray_index -> not_array_index

Done.

http://codereview.chromium.org/598062/diff/1/2#newcode9929
src/ia32/codegen-ia32.cc:9929: __ shl(c2, 8);
On 2010/02/12 10:24:05, Mads Ager wrote:
> kBitsInByte?

Done (kBitsPerByte).

http://codereview.chromium.org/598062/diff/1/2#newcode9952
src/ia32/codegen-ia32.cc:9952: // chars:        two character string (al first
char and ah second char)
On 2010/02/12 10:24:05, Mads Ager wrote:
> al and ah -> byte 0 and byte 1?  Nothing guarentees that the register will be
> eax, right?

Done.

http://codereview.chromium.org/598062/diff/1/2#newcode9985
src/ia32/codegen-ia32.cc:9985: // If length is not 2 the string is no a
candidate.
On 2010/02/12 10:24:05, Mads Ager wrote:
> no -> not

Done.

http://codereview.chromium.org/598062/diff/1/2#newcode10000
src/ia32/codegen-ia32.cc:10000: __ push(scratch);  // Save candidate.
On 2010/02/12 10:24:05, Mads Ager wrote:
> On the fast path we do push,pop,push,pop - can we make another entry label for
> pop_next_probe[i]?  That should be the right fall-through label, so I think it
> would be easy to avoid the extra push/pop?

Good point. Ended up pushing mask and used that as a temporary to keep the
candidate unchanged in a register.

http://codereview.chromium.org/598062/diff/1/3
File src/ia32/codegen-ia32.h (right):

http://codereview.chromium.org/598062/diff/1/3#newcode775
src/ia32/codegen-ia32.h:775: void
Generate2CharacterSymbolTableProbe(MacroAssembler* masm,
On 2010/02/12 10:24:05, Mads Ager wrote:
> 2 -> Two?

Done.

http://codereview.chromium.org/598062/diff/1/3#newcode781
src/ia32/codegen-ia32.h:781: Label *not_found);
On 2010/02/12 10:24:05, Mads Ager wrote:
> Label * -> Label*

Done.
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld 1f9106d