Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(65)

Issue 2218002: The way reloc entries are visited by the ObjectVisitor is architecture... (Closed)

Created:
10 years, 7 months ago by Erik Corry
Modified:
9 years, 7 months ago
CC:
v8-dev
Visibility:
Public.

Description

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

Patch Set 1 #

Total comments: 8

Patch Set 2 : '' #

Unified diffs Side-by-side diffs Delta from patch set Stats (+105 lines, -20 lines) Patch
M src/arm/assembler-arm-inl.h View 3 chunks +26 lines, -0 lines 0 comments Download
M src/assembler.h View 1 3 chunks +19 lines, -2 lines 0 comments Download
M src/ia32/assembler-ia32-inl.h View 3 chunks +26 lines, -0 lines 0 comments Download
M src/objects.cc View 1 chunk +1 line, -16 lines 0 comments Download
M src/serialize.cc View 2 chunks +2 lines, -2 lines 0 comments Download
M src/x64/assembler-x64-inl.h View 3 chunks +31 lines, -0 lines 0 comments Download

Messages

Total messages: 3 (0 generated)
Erik Corry
10 years, 7 months ago (2010-05-26 06:53:19 UTC) #1
Søren Thygesen Gjesse
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 ...
10 years, 7 months ago (2010-05-26 07:56:07 UTC) #2
Erik Corry
10 years, 7 months ago (2010-05-26 08:18:16 UTC) #3
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.

Powered by Google App Engine
This is Rietveld 408576698