Differentate between code target pointers and heap pointers in constant pools.

Differentate between code target pointers and heap pointers in constant pools.

Separate out code target pointers from normal heap pointer entries in constant
pool arrays so that the GC can correctly relocate these pointers using the
appropriate mechanism.

R=mstarzinger@chromium.org

Committed: https://code.google.com/p/v8/source/detail?r=19773

[rmcilroy, 2014-03-03 15:43:47]

This is what we discussed on Friday.  The main reason for this change is the
change in MarkCompactCollector::MigrateObjectI() to enable correct processing of
code entries in ConstantPoolArray objects.

PTAL

[Michael Starzinger, 2014-03-07 15:07:46]

Looking good. Just one concern about the initialization of the code entry
pointers within the constant pool.

https://codereview.chromium.org/183883011/diff/80001/src/heap.cc
File src/heap.cc (right):

https://codereview.chromium.org/183883011/diff/80001/src/heap.cc#newcode5312
src/heap.cc:5312: if (number_of_heap_ptr_entries > 0) {
Can we really skip initializing the code_ptr_entries here? IIUC they might be
visited before being set by the caller, so we should initialize. One valid
initialization value would be the entry point of the Builtins::kIllegal builtin.

https://codereview.chromium.org/183883011/diff/80001/src/objects-debug.cc
File src/objects-debug.cc (right):

https://codereview.chromium.org/183883011/diff/80001/src/objects-debug.cc#new...
src/objects-debug.cc:409: }
Can we also verify that all code_ptr entries point to code entry locations?

https://codereview.chromium.org/183883011/diff/80001/src/objects-visiting-inl.h
File src/objects-visiting-inl.h (right):

https://codereview.chromium.org/183883011/diff/80001/src/objects-visiting-inl...
src/objects-visiting-inl.h:493: int offset =  constant_pool->OffsetOfElementAt(
nit: Two white-spaces after equal sign.

https://codereview.chromium.org/183883011/diff/80001/src/objects-visiting-inl...
src/objects-visiting-inl.h:499: int offset =  constant_pool->OffsetOfElementAt(
nit: Two white-spaces after equal sign.

https://codereview.chromium.org/183883011/diff/80001/src/objects.cc
File src/objects.cc (right):

https://codereview.chromium.org/183883011/diff/80001/src/objects.cc#newcode9470
src/objects.cc:9470: int offset =  OffsetOfElementAt(first_code_ptr_index() +
i);
nit: Two white-spaces after equal sign.

https://codereview.chromium.org/183883011/diff/80001/src/objects.cc#newcode9475
src/objects.cc:9475: int offset =  OffsetOfElementAt(first_heap_ptr_index() +
i);
nit: Two white-spaces after equal sign.

[rmcilroy, 2014-03-10 14:03:05]

Addressed initialization and verification of code entrys - PTAL.

https://codereview.chromium.org/183883011/diff/80001/src/heap.cc
File src/heap.cc (right):

https://codereview.chromium.org/183883011/diff/80001/src/heap.cc#newcode5312
src/heap.cc:5312: if (number_of_heap_ptr_entries > 0) {
On 2014/03/07 15:07:47, Michael Starzinger wrote:
> Can we really skip initializing the code_ptr_entries here? IIUC they might be
> visited before being set by the caller, so we should initialize. One valid
> initialization value would be the entry point of the Builtins::kIllegal
builtin.

Yes I think you are right.  At the moment I don't think it's possible for a GC
to happen between a ConstantPoolArray being allocated and it's code target
entries being populated, but that might not always be the case.  Done (with
Builtins::kIllegal).

https://codereview.chromium.org/183883011/diff/80001/src/objects-debug.cc
File src/objects-debug.cc (right):

https://codereview.chromium.org/183883011/diff/80001/src/objects-debug.cc#new...
src/objects-debug.cc:409: }
On 2014/03/07 15:07:47, Michael Starzinger wrote:
> Can we also verify that all code_ptr entries point to code entry locations?

Done.

https://codereview.chromium.org/183883011/diff/80001/src/objects-visiting-inl.h
File src/objects-visiting-inl.h (right):

https://codereview.chromium.org/183883011/diff/80001/src/objects-visiting-inl...
src/objects-visiting-inl.h:493: int offset =  constant_pool->OffsetOfElementAt(
On 2014/03/07 15:07:47, Michael Starzinger wrote:
> nit: Two white-spaces after equal sign.

Done.

https://codereview.chromium.org/183883011/diff/80001/src/objects-visiting-inl...
src/objects-visiting-inl.h:499: int offset =  constant_pool->OffsetOfElementAt(
On 2014/03/07 15:07:47, Michael Starzinger wrote:
> nit: Two white-spaces after equal sign.

Done.

https://codereview.chromium.org/183883011/diff/80001/src/objects.cc
File src/objects.cc (right):

https://codereview.chromium.org/183883011/diff/80001/src/objects.cc#newcode9470
src/objects.cc:9470: int offset =  OffsetOfElementAt(first_code_ptr_index() +
i);
On 2014/03/07 15:07:47, Michael Starzinger wrote:
> nit: Two white-spaces after equal sign.

Done.

https://codereview.chromium.org/183883011/diff/80001/src/objects.cc#newcode9475
src/objects.cc:9475: int offset =  OffsetOfElementAt(first_heap_ptr_index() +
i);
On 2014/03/07 15:07:47, Michael Starzinger wrote:
> nit: Two white-spaces after equal sign.

Done.

[Michael Starzinger, 2014-03-10 15:00:37]

LGTM, just a few nits.

https://codereview.chromium.org/183883011/diff/100001/src/heap.h
File src/heap.h (right):

https://codereview.chromium.org/183883011/diff/100001/src/heap.h#newcode1013
src/heap.h:1013: int first_int64_index,
nit: For the sake of consistency could we prefix the argument names with
"number_of_" here as well? Otherwise someone reading just this declaration might
get confused.

https://codereview.chromium.org/183883011/diff/100001/src/objects-debug.cc
File src/objects-debug.cc (right):

https://codereview.chromium.org/183883011/diff/100001/src/objects-debug.cc#ne...
src/objects-debug.cc:410: CHECK(IsConstantPoolArray());
nit: Let's move this check to the top of the method.

https://codereview.chromium.org/183883011/diff/100001/src/objects-visiting-inl.h
File src/objects-visiting-inl.h (right):

https://codereview.chromium.org/183883011/diff/100001/src/objects-visiting-in...
src/objects-visiting-inl.h:493: int offset = constant_pool->OffsetOfElementAt(
nit: Can we use RawFieldOfElementAt he instead? That would be easier to read I
think.

https://codereview.chromium.org/183883011/diff/100001/src/objects-visiting-in...
src/objects-visiting-inl.h:499: int offset = constant_pool->OffsetOfElementAt(
nit: Can we use RawFieldOfElementAt he instead? That would be easier to read I
think.

https://codereview.chromium.org/183883011/diff/100001/src/objects.cc
File src/objects.cc (right):

https://codereview.chromium.org/183883011/diff/100001/src/objects.cc#newcode9470
src/objects.cc:9470: int offset = OffsetOfElementAt(first_code_ptr_index() + i);
nit: Can we use RawFieldOfElementAt he instead? That would be easier to read I
think.

https://codereview.chromium.org/183883011/diff/100001/src/objects.cc#newcode9475
src/objects.cc:9475: int offset = OffsetOfElementAt(first_heap_ptr_index() + i);
nit: Can we use RawFieldOfElementAt he instead? That would be easier to read I
think.

[rmcilroy, 2014-03-10 16:55:19]

https://codereview.chromium.org/183883011/diff/100001/src/heap.h
File src/heap.h (right):

https://codereview.chromium.org/183883011/diff/100001/src/heap.h#newcode1013
src/heap.h:1013: int first_int64_index,
On 2014/03/10 15:00:38, Michael Starzinger wrote:
> nit: For the sake of consistency could we prefix the argument names with
> "number_of_" here as well? Otherwise someone reading just this declaration
might
> get confused.

Oops good catch - thanks! Done

https://codereview.chromium.org/183883011/diff/100001/src/objects-debug.cc
File src/objects-debug.cc (right):

https://codereview.chromium.org/183883011/diff/100001/src/objects-debug.cc#ne...
src/objects-debug.cc:410: CHECK(IsConstantPoolArray());
On 2014/03/10 15:00:38, Michael Starzinger wrote:
> nit: Let's move this check to the top of the method.

Done.

https://codereview.chromium.org/183883011/diff/100001/src/objects-visiting-inl.h
File src/objects-visiting-inl.h (right):

https://codereview.chromium.org/183883011/diff/100001/src/objects-visiting-in...
src/objects-visiting-inl.h:493: int offset = constant_pool->OffsetOfElementAt(
On 2014/03/10 15:00:38, Michael Starzinger wrote:
> nit: Can we use RawFieldOfElementAt he instead? That would be easier to read I
> think.

Done.

https://codereview.chromium.org/183883011/diff/100001/src/objects-visiting-in...
src/objects-visiting-inl.h:499: int offset = constant_pool->OffsetOfElementAt(
On 2014/03/10 15:00:38, Michael Starzinger wrote:
> nit: Can we use RawFieldOfElementAt he instead? That would be easier to read I
> think.

Done.

https://codereview.chromium.org/183883011/diff/100001/src/objects.cc
File src/objects.cc (right):

https://codereview.chromium.org/183883011/diff/100001/src/objects.cc#newcode9470
src/objects.cc:9470: int offset = OffsetOfElementAt(first_code_ptr_index() + i);
On 2014/03/10 15:00:38, Michael Starzinger wrote:
> nit: Can we use RawFieldOfElementAt he instead? That would be easier to read I
> think.

Done.

https://codereview.chromium.org/183883011/diff/100001/src/objects.cc#newcode9475
src/objects.cc:9475: int offset = OffsetOfElementAt(first_heap_ptr_index() + i);
On 2014/03/10 15:00:38, Michael Starzinger wrote:
> nit: Can we use RawFieldOfElementAt he instead? That would be easier to read I
> think.

Done.

[rmcilroy, 2014-03-10 19:05:55]

Committed patchset #8 manually as r19773 (presubmit successful).