Special case the recording of constant pool entries in the slot buffer.

Special case the recording of constant pool entries in the slot buffer.

This CL enables RelocInfo pointers which live in the constant pool to be treated
as normal pointers by the slot buffer, avoiding the requirement of creating fake
RelocInfo objects during UpdateSlots() in order to update these slots.  This
is possible because constant pool entries are just pointers and don't require
the RelocInfo machinary to be updated.

EmbeddedObject constant pool entries can be added untyped to the slot buffer,
while code targets are still typed in order to correctly update the target
address based on the relocated code object.

Note: this is required in order to enable OOL constant pool support on Arm, but
should be benifitial for the current inline constant pool used by Arm code.

R=mstarzinger@chromium.org

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

[rmcilroy, 2014-02-25 17:25:00]

Michael: This is what we talked about when I was in Munich - avoiding having to
"fake-up" RelocInfo objects for UpdateSlots().  This is required for the OOL
constant pool work, but should benefit Arm's current inline constant pool too. 
PTAL.

[rmcilroy, 2014-03-03 15:44:00]

On 2014/02/25 17:25:00, rmcilroy wrote:
> Michael: This is what we talked about when I was in Munich - avoiding having
to
> "fake-up" RelocInfo objects for UpdateSlots().  This is required for the OOL
> constant pool work, but should benefit Arm's current inline constant pool too.

> PTAL.

Ping?

[Michael Starzinger, 2014-03-07 14:36:59]

LGTM, only nits and naming.

https://codereview.chromium.org/179813005/diff/1/src/assembler.h
File src/assembler.h (right):

https://codereview.chromium.org/179813005/diff/1/src/assembler.h#newcode415
src/assembler.h:415: INLINE(Address constant_pool_entry_address());
nit: Let's add an empty newline for readability after the declaration.

https://codereview.chromium.org/179813005/diff/1/src/liveedit.cc
File src/liveedit.cc (right):

https://codereview.chromium.org/179813005/diff/1/src/liveedit.cc#newcode1028
src/liveedit.cc:1028: *reinterpret_cast<Address*>(target_address_pointer) =
substitution_entry;
Can we instead of the reinterpret cast use ...

Memory::Address_at(target_address_pointer) = substitution_entry;

https://codereview.chromium.org/179813005/diff/1/src/mark-compact.cc
File src/mark-compact.cc (right):

https://codereview.chromium.org/179813005/diff/1/src/mark-compact.cc#newcode2923
src/mark-compact.cc:2923: *reinterpret_cast<Address*>(target_address_pointer) =
Instead of the reinterpret case, case we use ...

Memory::Address_at(target_address_pointer) =
    Code::cast(target)->instruction_start();

https://codereview.chromium.org/179813005/diff/1/src/objects.cc
File src/objects.cc (right):

https://codereview.chromium.org/179813005/diff/1/src/objects.cc#newcode10262
src/objects.cc:10262: void ObjectVisitor::VisitCodeTargetCommon(Address
target_address) {
IMHO, the logic in here is small enough so that we can inline into the two
callers and get rid of the common helper. I think that's easier to read.

https://codereview.chromium.org/179813005/diff/1/src/objects.h
File src/objects.h (right):

https://codereview.chromium.org/179813005/diff/1/src/objects.h#newcode10702
src/objects.h:10702: virtual void VisitCodeTarget(Address
target_address_pointer);
Can we rename this to method to make it explicit that it is a constant cool
entry. Maybe "VisitConstantPoolCodeTarget" or "VisitCodeTargetInConstantPool"?

https://codereview.chromium.org/179813005/diff/1/src/objects.h#newcode10747
src/objects.h:10747: void VisitCodeTargetCommon(Address target_address);
See comment in the .cc file.

[Michael Starzinger, 2014-03-07 15:34:04]

More thoughts ...

https://codereview.chromium.org/179813005/diff/1/src/objects.h
File src/objects.h (right):

https://codereview.chromium.org/179813005/diff/1/src/objects.h#newcode10702
src/objects.h:10702: virtual void VisitCodeTarget(Address
target_address_pointer);
On 2014/03/07 14:36:59, Michael Starzinger wrote:
> Can we rename this to method to make it explicit that it is a constant cool
> entry. Maybe "VisitConstantPoolCodeTarget" or "VisitCodeTargetInConstantPool"?

Actually, the more I think about it: Is there any semantic difference between
the new method and the existing VisitCodeEntry()? Could we reuse that instead?
IIUC they should always behave the same, right?

[rmcilroy, 2014-03-10 12:25:22]

https://codereview.chromium.org/179813005/diff/1/src/assembler.h
File src/assembler.h (right):

https://codereview.chromium.org/179813005/diff/1/src/assembler.h#newcode415
src/assembler.h:415: INLINE(Address constant_pool_entry_address());
On 2014/03/07 14:36:59, Michael Starzinger wrote:
> nit: Let's add an empty newline for readability after the declaration.

Done (also for target_address_address declaration below).

https://codereview.chromium.org/179813005/diff/1/src/liveedit.cc
File src/liveedit.cc (right):

https://codereview.chromium.org/179813005/diff/1/src/liveedit.cc#newcode1028
src/liveedit.cc:1028: *reinterpret_cast<Address*>(target_address_pointer) =
substitution_entry;
On 2014/03/07 14:36:59, Michael Starzinger wrote:
> Can we instead of the reinterpret cast use ...
> 
> Memory::Address_at(target_address_pointer) = substitution_entry;

Removed VisitCodeTarget(Address) as discussed, so no longer applicable.

https://codereview.chromium.org/179813005/diff/1/src/mark-compact.cc
File src/mark-compact.cc (right):

https://codereview.chromium.org/179813005/diff/1/src/mark-compact.cc#newcode2923
src/mark-compact.cc:2923: *reinterpret_cast<Address*>(target_address_pointer) =
On 2014/03/07 14:36:59, Michael Starzinger wrote:
> Instead of the reinterpret case, case we use ...
> 
> Memory::Address_at(target_address_pointer) =
>     Code::cast(target)->instruction_start();

Removed VisitCodeTarget(Address) as discussed, so no longer applicable.

https://codereview.chromium.org/179813005/diff/1/src/objects.cc
File src/objects.cc (right):

https://codereview.chromium.org/179813005/diff/1/src/objects.cc#newcode10262
src/objects.cc:10262: void ObjectVisitor::VisitCodeTargetCommon(Address
target_address) {
On 2014/03/07 14:36:59, Michael Starzinger wrote:
> IMHO, the logic in here is small enough so that we can inline into the two
> callers and get rid of the common helper. I think that's easier to read.

Removed VisitCodeTarget(Address) as discussed, so no longer applicable.

https://codereview.chromium.org/179813005/diff/1/src/objects.h
File src/objects.h (right):

https://codereview.chromium.org/179813005/diff/1/src/objects.h#newcode10702
src/objects.h:10702: virtual void VisitCodeTarget(Address
target_address_pointer);
On 2014/03/07 15:34:04, Michael Starzinger wrote:
> On 2014/03/07 14:36:59, Michael Starzinger wrote:
> > Can we rename this to method to make it explicit that it is a constant cool
> > entry. Maybe "VisitConstantPoolCodeTarget" or
"VisitCodeTargetInConstantPool"?
> 
> Actually, the more I think about it: Is there any semantic difference between
> the new method and the existing VisitCodeEntry()? Could we reuse that instead?
> IIUC they should always behave the same, right?

Yes you are right, this is much better - done.  

As an aside, the CodeEntry/CodeTarget differentiation seems confusingly named -
I didn't realize that CodeEntry and CodeTarget effectively did the same thing
just from either and Address or RelocInfo pointer...

https://codereview.chromium.org/179813005/diff/1/src/objects.h#newcode10747
src/objects.h:10747: void VisitCodeTargetCommon(Address target_address);
On 2014/03/07 14:36:59, Michael Starzinger wrote:
> See comment in the .cc file.

Removed VisitCodeTarget(Address) as discussed, so no longer applicable.

[Michael Starzinger, 2014-03-10 12:59:19]

Much cleaner. I like it. LGTM.

[rmcilroy, 2014-03-10 18:48:10]

Committed patchset #2 manually as r19772 (presubmit successful).