http://codereview.chromium.org/3158020/diff/1/2 File src/arm/codegen-arm.cc (right):
http://codereview.chromium.org/3158020/diff/1/2#newcode5301 src/arm/codegen-arm.cc:5301: __ ldm(ib, r0, r3.bit() | r4.bit() | r5.bit() | r6.bit() | r7.bit()); Added check for EmptyFixedArray. http://codereview.chromium.org/3158020/diff/1/2#newcode5308 src/arm/codegen-arm.cc:5308: __ str(ip, FieldMemOperand(r4, HeapObject::kMapOffset)); This was deliberate, to give the load from the root array an extra cycle to complete. I've moved the add further up to help another load instead. http://codereview.chromium.org/3158020/diff/1/9 File src/ia32/codegen-ia32.cc (right): http://codereview.chromium.org/3158020/diff/1/9#newcode5555 src/ia32/codegen-ia32.cc:5555: __ push(save_register); Amazing that it appears to work! http://codereview.chromium.org/3158020/diff/1/9#newcode7402 src/ia32/codegen-ia32.cc:7402: __ cmp(edx, FieldOperand(eax, HeapObject::kMapOffset)); Will do. http://codereview.chromium.org/3158020/diff/1/9#newcode7421 src/ia32/codegen-ia32.cc:7421: // Copy properties array of input (should be empty FixedArray). I can do the loop, with a few special cases, and even unroll it for speed (my experience is that copying two values at a time is faster than one at a time, although I admit that I haven't tested it here.). As for the empty array - it shouldn't be possible, because the input is a regexp result which cannot be empty (in that case it just returns null). However "shouldn't be possible" isn't enough for a runtime function. I'll add a sanity check. http://codereview.chromium.org/3158020/diff/1/13 File src/runtime.cc (right): http://codereview.chromium.org/3158020/diff/1/13#newcode1378 src/runtime.cc:1378: if (result->map() != regexp_result_map) { Fixed. http://codereview.chromium.org/3158020/diff/1/13#newcode1379 src/runtime.cc:1379: return result; Done. Answer: Argument is either null or an unmodified JSRegExpResult - unless someone is tampering with the runtime system. This is both a null-test and a sanity check. http://codereview.chromium.org/3158020/diff/1/13#newcode1382 src/runtime.cc:1382: ASSERT(result->HasFastElements()); Done. http://codereview.chromium.org/3158020/diff/1/13#newcode1387 src/runtime.cc:1387: NEW_SPACE, Done. http://codereview.chromium.org/3158020/diff/1/13#newcode1397 src/runtime.cc:1397: new_array->set_properties(result->properties()); // Is empty FixedArray. Done. http://codereview.chromium.org/3158020/diff/1/13#newcode1400 src/runtime.cc:1400: FixedArray* elements = FixedArray::cast(result->elements()); That is, sadly, not guaranteed by the map check, only by the intended use of the function. Since this is a runtime function, an assertion isn't enough. I'll add a guard on the setting-cow-map (which is safe, because having the EmptyFixedArray as elements is effectively copy-on-write). http://codereview.chromium.org/3158020/diff/1/13#newcode1403 src/runtime.cc:1403: new_array->set_elements(result->elements()); // Is empty FixedArray. Done. http://codereview.chromium.org/3158020/diff/1/15 File src/x64/codegen-x64.cc (right): http://codereview.chromium.org/3158020/diff/1/15#newcode4834 src/x64/codegen-x64.cc:4834: __ push(save_register); Doh. http://codereview.chromium.org/3158020/diff/1/15#newcode6637 src/x64/codegen-x64.cc:6637: // Load JSRegExp map into edx. Check that argument object has this map. Fixed. http://codereview.chromium.org/3158020/diff/1/15#newcode6662 src/x64/codegen-x64.cc:6662: // Copy properties array of input (should be empty FixedArray). Changed to match the new ia32 code. http://codereview.chromium.org/3158020/diff/1/18 File test/mjsunit/regexp.js (right): http://codereview.chromium.org/3158020/diff/1/18#newcode488 test/mjsunit/regexp.js:488: // Test that caching of result doesn't share result objects. Done. http://codereview.chromium.org/3158020/show -- v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev
