http://codereview.chromium.org/6248004/diff/6001/src/arm/code-stubs-arm.cc File src/arm/code-stubs-arm.cc (right):
http://codereview.chromium.org/6248004/diff/6001/src/arm/code-stubs-arm.cc#newcode2893 src/arm/code-stubs-arm.cc:2893: // Uses registers r0 to r4. Expected input is Please update this comment to include information on the possible patching, the structure of the code being patched, and where the patching delta is found. Also args_in_registers() is now called HasArgsInRegisters(). I suggest using wording like "args in registers" instead of function names in comments when possible. http://codereview.chromium.org/6248004/diff/6001/src/arm/code-stubs-arm.cc#newcode2963 src/arm/code-stubs-arm.cc:2963: __ ldr(scratch, MemOperand(inline_site)); This code pattern (the patching of the constant pool entry) is duplicated three times. Please refactor this into either the macro assembler or a helper generator function on the InstanceofStub. http://codereview.chromium.org/6248004/diff/6001/src/arm/code-stubs-arm.cc#newcode2991 src/arm/code-stubs-arm.cc:2991: Register scratch3 = map; Please add map = no_reg; as well. http://codereview.chromium.org/6248004/diff/6001/src/arm/lithium-codegen-arm.cc File src/arm/lithium-codegen-arm.cc (right): http://codereview.chromium.org/6248004/diff/6001/src/arm/lithium-codegen-arm.cc#newcode1628 src/arm/lithium-codegen-arm.cc:1628: // We use Factory::the_hole_value() on purpose to force relocation to be Extend comment with "instead of loading from the root array" (I assume that is what you mean). http://codereview.chromium.org/6248004/diff/6001/src/arm/lithium-codegen-arm.cc#newcode1633 src/arm/lithium-codegen-arm.cc:1633: // We use Factory::the_hole_value() on purpose to force relocation to be Ditto. http://codereview.chromium.org/6248004/diff/6001/src/arm/lithium-codegen-arm.cc#newcode1684 src/arm/lithium-codegen-arm.cc:1684: static const int kAdditionalDelta = 4; // 3 instructions + 1 for reloc info. I think the 4 here is 4 instructions Call always generates two instructions, and the reloc info goes in the constant pool later. Regarding the constant pool I am quite sure that we need to block the constant pool for for these 4 instructions. Otherwise we might potentially get a constant pool inside this sequence and the calculation of the patch site will be wrong. http://codereview.chromium.org/6248004/diff/6001/src/arm/lithium-codegen-arm.cc#newcode1688 src/arm/lithium-codegen-arm.cc:1688: __ mov(temp, Operand(delta * kPointerSize)); On IA32 we have EspIndexForPushAll, and I think we need something like that here, maybe a macro assembler instruction like StoreToSafePointRegister to work in correspondance with PushSafepointRegisters/PopSafepointRegisters. [email protected] is also working on the safepoint stuff adding saving of doubles, see http://codereview.chromium.org/6164005/. http://codereview.chromium.org/6248004/ -- v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev
