Comments addressed, CopyBytes function factored out, and optimized.
http://codereview.chromium.org/6148007/diff/6001/src/ia32/full-codegen-ia32.cc File src/ia32/full-codegen-ia32.cc (right): http://codereview.chromium.org/6148007/diff/6001/src/ia32/full-codegen-ia32.cc#newcode3359 src/ia32/full-codegen-ia32.cc:3359: loop_3b, loop_3b_condition; On 2011/01/13 09:20:32, Lasse Reichstein wrote:
That's a lot of loops. Could they be given more telling names? Or give
an
explanation of the algorithm here, so we can see which loops do what.
Loops eliminated by making the string copy a macro-assembler function. http://codereview.chromium.org/6148007/diff/6001/src/ia32/full-codegen-ia32.cc#newcode3398 src/ia32/full-codegen-ia32.cc:3398: 1 << Map::kHasFastElements); On 2011/01/13 09:20:32, Lasse Reichstein wrote:
Will Copy-on-Write arrays have fast elements if queried this way?
Yes, they will (I checked). http://codereview.chromium.org/6148007/diff/6001/src/ia32/full-codegen-ia32.cc#newcode3422 src/ia32/full-codegen-ia32.cc:3422: // Check that all array elements are sequential ascii strings, and On 2011/01/13 09:20:32, Lasse Reichstein wrote:
ascii -> ASCII.
Both forms occur in our comments, about 1/3 lowercase, 2/3 uppercase. I prefer lowercase. But done. http://codereview.chromium.org/6148007/diff/6001/src/ia32/full-codegen-ia32.cc#newcode3423 src/ia32/full-codegen-ia32.cc:3423: // accumulate the sum of their lengths. On 2011/01/13 09:20:32, Lasse Reichstein wrote:
Say that the sum is maintained as a Smi value.
Done. http://codereview.chromium.org/6148007/diff/6001/src/ia32/full-codegen-ia32.cc#newcode3440 src/ia32/full-codegen-ia32.cc:3440: __ mov_b(scratch, FieldOperand(scratch, Map::kInstanceTypeOffset)); On 2011/01/13 09:20:32, Lasse Reichstein wrote:
Use movzx_b to avoid depending on the remaining bits, or use and_b and
cmp_b
below.
done. http://codereview.chromium.org/6148007/diff/6001/src/ia32/full-codegen-ia32.cc#newcode3472 src/ia32/full-codegen-ia32.cc:3472: // Check that the separator is a flat ascii string. On 2011/01/13 09:20:32, Lasse Reichstein wrote:
ASCII.
Done. http://codereview.chromium.org/6148007/diff/6001/src/ia32/full-codegen-ia32.cc#newcode3487 src/ia32/full-codegen-ia32.cc:3487: __ sub(current_string_length, Operand(scratch)); On 2011/01/13 09:20:32, Lasse Reichstein wrote:
Perhaps worth noting that this can (and may) give a negative result.
Done. http://codereview.chromium.org/6148007/ -- v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev
