The way reloc entries are visited by the ObjectVisitor is architecture...

The way reloc entries are visited by the ObjectVisitor is architecture
dependent, so we push it down to the architecture dependent files.
Currently all architectures visit in almost the same way, but this is
about to change on ARM with movw/movt.

Committed: http://code.google.com/p/v8/source/detail?r=4721

[Søren Thygesen Gjesse, 2010-05-26 07:56:07]

LGTM

http://codereview.chromium.org/2218002/diff/1/3
File src/assembler.h (right):

http://codereview.chromium.org/2218002/diff/1/3#newcode203
src/assembler.h:203: // Read the address of the word containing the
target_address.  This is
I don't fully understand "This is used by the serializer to find out how many
raw bytes of instruction to output before the next target." That is only when
target_address_size() returns zero?

http://codereview.chromium.org/2218002/diff/1/3#newcode209
src/assembler.h:209: // be zero for architectures where the target is patched
into some already
I think this comment should be more clear. The wording "will generally be zero"
led me to believe that that was the "normal" case currently in use which it is
not, as the actual implementations never return zero.

Also should there not be a constant for that zero (for future use).

http://codereview.chromium.org/2218002/diff/1/3#newcode211
src/assembler.h:211: // function above should return the end of the instructions
to be patched,
Why end of the instructions and not start?

http://codereview.chromium.org/2218002/diff/1/3#newcode229
src/assembler.h:229: inline void Visit(ObjectVisitor* v);
Does this really belong in the assembler? How about objects-<arch>.cc?

http://codereview.chromium.org/2218002/diff/1/7
File src/x64/assembler-x64-inl.h (right):

http://codereview.chromium.org/2218002/diff/1/7#newcode234
src/x64/assembler-x64-inl.h:234: if (IsCodedSpecially()) {
I know the names Assembler::kCallTargetSize and Assembler::kExternalTargetSize
are not new, but I find them somewhat confusing.

[Erik Corry, 2010-05-26 08:18:16]

http://codereview.chromium.org/2218002/diff/1/3
File src/assembler.h (right):

http://codereview.chromium.org/2218002/diff/1/3#newcode203
src/assembler.h:203: // Read the address of the word containing the
target_address.  This is
On 2010/05/26 07:56:07, Søren Gjesse wrote:
> I don't fully understand "This is used by the serializer to find out how many
> raw bytes of instruction to output before the next target." That is only when
> target_address_size() returns zero?

I can see that these comments were confusing.  I have clarified them in the new
version.

http://codereview.chromium.org/2218002/diff/1/3#newcode229
src/assembler.h:229: inline void Visit(ObjectVisitor* v);
On 2010/05/26 07:56:07, Søren Gjesse wrote:
> Does this really belong in the assembler? How about objects-<arch>.cc?

It's all about manipulating the pointers embedded in the instruction stream
created by the assembler, so I think it fits well here.  Since it is rather
performance critical on some workloads (GC, mainly) I am reluctant to add
abstraction layers here.

http://codereview.chromium.org/2218002/diff/1/7
File src/x64/assembler-x64-inl.h (right):

http://codereview.chromium.org/2218002/diff/1/7#newcode234
src/x64/assembler-x64-inl.h:234: if (IsCodedSpecially()) {
On 2010/05/26 07:56:07, Søren Gjesse wrote:
> I know the names Assembler::kCallTargetSize and Assembler::kExternalTargetSize
> are not new, but I find them somewhat confusing.

Indeed.  I hope to clear this up in a later change.