http://codereview.chromium.org/7477045/diff/52001/src/arm/code-stubs-arm.cc File src/arm/code-stubs-arm.cc (right):
http://codereview.chromium.org/7477045/diff/52001/src/arm/code-stubs-arm.cc#newcode4343 src/arm/code-stubs-arm.cc:4343: __ mov(r9, Operand(0)); does it belong here? maybe move it int http://codereview.chromium.org/7477045/diff/52001/src/arm/code-stubs-arm.cc#newcode4456 src/arm/code-stubs-arm.cc:4456: __ add(r8, subject, Operand(SeqAsciiString::kHeaderSize - kHeapObjectTag)); Apparently if you do: __ add(r8, subject, Operand(...)); __ add(r9, r8, Operand(r9, LSL, r3)); (of course, you need to swap with __ eor below) you can simplify calculations below. Start would be just __ add(r2, r9, Operand(r1, LSL, r3)); (as before) And you won't need to do __ add(r8, r8, Operand(...)) at line 4470 in the patched code. WDYT? http://codereview.chromium.org/7477045/diff/52001/src/arm/code-stubs-arm.cc#newcode4462 src/arm/code-stubs-arm.cc:4462: __ ldr(r0, MemOperand(fp, kSubjectOffset + 2*kPointerSize)); nit: spaces around * (and in comment) http://codereview.chromium.org/7477045/diff/52001/src/arm/code-stubs-arm.cc#newcode4462 src/arm/code-stubs-arm.cc:4462: __ ldr(r0, MemOperand(fp, kSubjectOffset + 2*kPointerSize)); this looks complicated to me. Cannot you thread original subject in some other register? Just asking. http://codereview.chromium.org/7477045/diff/52001/src/arm/code-stubs-arm.cc#newcode4463 src/arm/code-stubs-arm.cc:4463: // If slice offset is not 0, load the length from the original sliced string. I don't see any conditional logic below. http://codereview.chromium.org/7477045/diff/52001/src/arm/code-stubs-arm.cc#newcode4863 src/arm/code-stubs-arm.cc:4863: __ ldrb(result_, FieldMemOperand(result_, Map::kInstanceTypeOffset)); might be worth asserting that result_ is a flat string. http://codereview.chromium.org/7477045/diff/52001/src/arm/code-stubs-arm.cc#newcode5965 src/arm/code-stubs-arm.cc:5965: STATIC_ASSERT(SlicedString::kMinLength
= String::kMinNonFlatLength);
again, might be worth asserting. http://codereview.chromium.org/7477045/diff/52001/src/arm/lithium-codegen-arm.cc File src/arm/lithium-codegen-arm.cc (left): http://codereview.chromium.org/7477045/diff/52001/src/arm/lithium-codegen-arm.cc#oldcode3507 src/arm/lithium-codegen-arm.cc:3507: __ ldrb(result, FieldMemOperand(string, why const operand optimization is gone away? Yes, implementing it for sliced is more involved, but still it seems worth it. http://codereview.chromium.org/7477045/diff/52001/src/arm/lithium-codegen-arm.cc File src/arm/lithium-codegen-arm.cc (right): http://codereview.chromium.org/7477045/diff/52001/src/arm/lithium-codegen-arm.cc#newcode3472 src/arm/lithium-codegen-arm.cc:3472: // shapes we support have just been unwrapped above. again, it might be a good idea to check it in debug mode. Also, we can never have Cons w/ 1st sliced component and empty second component, correct? http://codereview.chromium.org/7477045/diff/52001/src/heap.cc File src/heap.cc (right): http://codereview.chromium.org/7477045/diff/52001/src/heap.cc#newcode2648 src/heap.cc:2648: if (!FLAG_string_slices || this condition is very hard to parse. http://codereview.chromium.org/7477045/diff/52001/src/heap.cc#newcode2684 src/heap.cc:2684: MaybeObject* maybe_result = Allocate(map, NEW_SPACE); why ignore pretenure flag here? http://codereview.chromium.org/7477045/diff/52001/src/ia32/code-stubs-ia32.cc File src/ia32/code-stubs-ia32.cc (right): http://codereview.chromium.org/7477045/diff/52001/src/ia32/code-stubs-ia32.cc#newcode3433 src/ia32/code-stubs-ia32.cc:3433: // edx: map of first part of cons string or map of parent of sliced string. do you use edx below? apparently not, but if yes, you doesn't set it as promised for sliced strings. http://codereview.chromium.org/7477045/diff/52001/src/ia32/code-stubs-ia32.cc#newcode3522 src/ia32/code-stubs-ia32.cc:3522: __ add(ebx, Operand(edi)); // Calculate input start wrt offset. please, document what's in ebx. http://codereview.chromium.org/7477045/ -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev
