LGTM if it works (I don't have complete understanding of how the relocation works).
http://codereview.chromium.org/200095/diff/10001/10010 File src/arm/assembler-arm-inl.h (right): http://codereview.chromium.org/200095/diff/10001/10010#newcode84 Line 84: return Memory::Object_at(Assembler::target_address_address_at(pc_)); Can't we find a better name than "address_address"? http://codereview.chromium.org/200095/diff/10001/10012 File src/serialize.cc (right): http://codereview.chromium.org/200095/diff/10001/10012#newcode943 Line 943: intptr_t int_target = reinterpret_cast<intptr_t>(encoded_target); int_target is a bad name for something that isn't an int. How about "scalar_target", or "target_as_scalar", or just "intptr_target". http://codereview.chromium.org/200095/diff/10001/10012#newcode944 Line 944: int_target = (int_target & 0xFFFFFFFFU) | (Memory::uint64_at(rinfo->pc() + 4) << 32); This needs a comment! I.e., I have no idea what is going on. Can't you get rid of the uint64_at thing in 32-bit mode? http://codereview.chromium.org/200095/diff/10001/10012#newcode1670 Line 1670: encoded = reinterpret_cast<Address>(0xFFFFFFFFU & reinterpret_cast<intptr_t>(encoded)); The content of "encoded" isn't really an Address (it's not a pointer to a byte in memory if it's always at most 32 bit). Maybe it should have a different type. http://codereview.chromium.org/200095/diff/10001/10008 File src/x64/assembler-x64-inl.h (right): http://codereview.chromium.org/200095/diff/10001/10008#newcode284 Line 284: pc_ + Assembler::kRealPatchReturnSequenceAddressOffset); "Real" is not a good prefix for a variable. It doesn't explain how kPatchReturnSequenceAddressOffset isn't real. http://codereview.chromium.org/200095/diff/10001/10006 File src/x64/assembler-x64.cc (right): http://codereview.chromium.org/200095/diff/10001/10006#newcode267 Line 267: Assembler::Assembler(void* buffer, int buffer_size): Move ":" to next line or initializer to this line. http://codereview.chromium.org/200095/diff/10001/10007 File src/x64/assembler-x64.h (right): http://codereview.chromium.org/200095/diff/10001/10007#newcode940 Line 940: void jmp(Handle<Code> target, RelocInfo::Mode rmode); On a side note, I would like to move all Handle operations to the Macro-assembler, so that the assembler doesn't need to know about the domain model. I.e., all handle and smi operations should be in the macro assembler, and reduce to assembler operations on simple values. http://codereview.chromium.org/200095 --~--~---------~--~----~------------~-------~--~----~ v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev -~----------~----~----~----~------~----~------~--~---
