Thanks a lot for working on this!
http://codereview.chromium.org/6462029/diff/1/src/arm/simulator-arm.cc File src/arm/simulator-arm.cc (right): http://codereview.chromium.org/6462029/diff/1/src/arm/simulator-arm.cc#newcode1534 src/arm/simulator-arm.cc:1534: typedef v8::Handle<v8::Value> (*SimulatorRuntimeDirectApiCall)(int32_t arg0); Do we need word Direct here? It looks like simulator doesn't have to know it's anything direct. But if you feel it conveys important semantic thing, of course we should keep it. http://codereview.chromium.org/6462029/diff/1/src/arm/stub-cache-arm.cc File src/arm/stub-cache-arm.cc (right): http://codereview.chromium.org/6462029/diff/1/src/arm/stub-cache-arm.cc#newcode1242 src/arm/stub-cache-arm.cc:1242: Register holder_reg = if you change this, please, change other platforms files as well: we're trying to keep the things in parallel as much as we can, http://codereview.chromium.org/6462029/diff/1/src/arm/stub-cache-arm.cc#newcode1245 src/arm/stub-cache-arm.cc:1245: ASSERT(holder_reg.is(receiver) || holder_reg.is(scratch1)); do you waht to assert this or rather the fact that !holder_reg.is(scratch{2,3})? http://codereview.chromium.org/6462029/diff/1/src/arm/stub-cache-arm.cc#newcode1249 src/arm/stub-cache-arm.cc:1249: __ push(receiver); may I ask you to reformat and comment you code slightly to better communicate what you're doing here? I'd suggest: Comment at ln. 1247 becomes: // Build AccessorGetter arguments on the stack below exit frame to make GC aware of them and store pointers to them. [Empty line] // Build AccessorInfo on the stack. __ push(receiver); __ mov(scratch2, sp); // scratch2 = &AccessorInfo. __ push(holder_reg); .... __ push(sratch3); [Here comes blank line] // Push name to the stack. __ push(name_reg); __ mov(r0, sp); http://codereview.chromium.org/6462029/diff/1/src/arm/stub-cache-arm.cc#newcode1268 src/arm/stub-cache-arm.cc:1268: __ str(scratch2, MemOperand(sp, 1 * kPointerSize)); Please, explain briefly why we need additional wrapping here. BTW, is this ABI universal across ARM compilers? http://codereview.chromium.org/6462029/diff/1/src/assembler.h File src/assembler.h (right): http://codereview.chromium.org/6462029/diff/1/src/assembler.h#newcode481 src/assembler.h:481: DIRECT_API_CALL, again question about DIRECT, maybe just drop it, but again no insisting, feel free to keep it if you like http://codereview.chromium.org/6462029/diff/1/src/assembler.h#newcode485 src/assembler.h:485: DIRECT_LOAD_CALL Please, do not use load and use getter or callback instead (or even accessor getter): we already have this callback/getter distinction and I think we should introduce another word. http://codereview.chromium.org/6462029/diff/1/test/cctest/test-api.cc File test/cctest/test-api.cc (right): http://codereview.chromium.org/6462029/diff/1/test/cctest/test-api.cc#newcode7587 test/cctest/test-api.cc:7587: v8::V8::LowMemoryNotification(); you can probably directly force GC as v8::internal::Heap is exposed here. And yes, we do it :) http://codereview.chromium.org/6462029/diff/1/test/cctest/test-api.cc#newcode7628 test/cctest/test-api.cc:7628: CHECK_EQ(v8_str("ggggg"), result); Thank you very much for the tests! http://codereview.chromium.org/6462029/ -- v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev
