Added more tests in cctest/test-strings/ExternalShortStringAdd adding internal and external strings.
http://codereview.chromium.org/442024/diff/2001/3002 File src/ia32/codegen-ia32.cc (right): http://codereview.chromium.org/442024/diff/2001/3002#newcode7059 src/ia32/codegen-ia32.cc:7059: // First and second argument are strings jump to the string add stub. On 2009/11/27 08:13:54, Mads Ager wrote: > "... are strings. Jump to the ..."? Done. http://codereview.chromium.org/442024/diff/2001/3002#newcode8242 src/ia32/codegen-ia32.cc:8242: __ j(equal, &string_add_runtime); On 2009/11/27 14:29:57, Erik Corry wrote: > Did you check that this is worth it? There is no measurable difference between calling to the runtime for two one character strings and not. I will leave it like this and do some more experiments on the handling of two char strings. http://codereview.chromium.org/442024/diff/2001/3002#newcode8247 src/ia32/codegen-ia32.cc:8247: __ cmp(ebx, String::kMaxLength); On 2009/11/27 14:29:57, Erik Corry wrote: > Perhaps assert here that kMaxLength*2 fits in 32 bits. Done. http://codereview.chromium.org/442024/diff/2001/3002#newcode8298 src/ia32/codegen-ia32.cc:8298: __ add(Operand(ecx), Immediate(SeqAsciiString::kHeaderSize - kHeapObjectTag)); On 2009/11/27 14:29:57, Erik Corry wrote: > What about external strings? Good catch. Added check for external strings and handle this in the runtime system. http://codereview.chromium.org/442024/diff/2001/3002#newcode8365 src/ia32/codegen-ia32.cc:8365: void StringAddStub::GenerateCopyAsciiCharacters(MacroAssembler* masm, On 2009/11/27 08:13:54, Mads Ager wrote: > You could have just one GenerateCopyCharacters and pass it an argument > indicating whether the string is ascii or two-byte. Done. http://codereview.chromium.org/442024/diff/2001/3002#newcode8365 src/ia32/codegen-ia32.cc:8365: void StringAddStub::GenerateCopyAsciiCharacters(MacroAssembler* masm, On 2009/11/27 08:40:16, Lasse Reichstein wrote: > This seems inefficient at first, moving one byte at a time, but since its only > used for small counts (<13), it's probably not worth changing. > Could you add a comment saying that count is usually low? Added comment. http://codereview.chromium.org/442024/diff/2001/3003 File src/ia32/codegen-ia32.h (right): http://codereview.chromium.org/442024/diff/2001/3003#newcode773 src/ia32/codegen-ia32.h:773: bool string_check_; // Should the stub check whether arguments are strings? On 2009/11/27 08:13:54, Mads Ager wrote: > Put the comment on the line before? Done. http://codereview.chromium.org/442024 -- v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev
