I'm getting close to resubmitting this as discussed. See responses to prior
review comments.
https://codereview.chromium.org/1030353003/diff/1/src/compiler/code-generator.cc
File src/compiler/code-generator.cc (right):
https://codereview.chromium.org/1030353003/diff/1/src/compiler/code-generator.cc#newcode125
src/compiler/code-generator.cc:125: FinishCode(masm());
On 2015/04/08 12:38:55, rmcilroy (OOO till 18th May) wrote:
Don't move this - instead insert a separate step which emits the
embedded
constant pool.
Acknowledged.
https://codereview.chromium.org/1030353003/diff/1/src/frames.cc
File src/frames.cc (left):
https://codereview.chromium.org/1030353003/diff/1/src/frames.cc#oldcode326
src/frames.cc:326: }
On 2015/04/08 12:38:55, rmcilroy (OOO till 18th May) wrote:
I'm not sure why you are making this change here, could you explain?
By definition, the an exit frame's pc_address is above sp (see
ExitFrame::FillState). Thus, this check always fails.
https://codereview.chromium.org/1030353003/diff/1/src/ic/ic.h
File src/ic/ic.h (right):
https://codereview.chromium.org/1030353003/diff/1/src/ic/ic.h#newcode291
src/ic/ic.h:291: };
On 2015/04/08 12:38:55, rmcilroy (OOO till 18th May) wrote:
let's not make this a union. I'm not sure that storing this as a raw
address is
OK - what if a GC happens in the middle of the IC handling and the
code object
movede, then this address will point to the wrong location.
This address is the stack slot holding the constant pool pointer -- not
the constant pool pointer itself (providing a level of indirection just
like pc_address_). I will make this more clear.
https://codereview.chromium.org/1030353003/diff/1/src/objects.h
File src/objects.h (right):
https://codereview.chromium.org/1030353003/diff/1/src/objects.h#newcode5628
src/objects.h:5628: static const int kOOLConstantPoolOffset =
kPrologueOffset + kIntSize;
On 2015/04/08 12:38:55, rmcilroy (OOO till 18th May) wrote:
Let's just have a single field here which points to the embedded
constant pool
(since we are removing the OOL constant pool support) - this should
avoid you
having to do the extra tristate things below.
Acknowledged.
https://codereview.chromium.org/1030353003/diff/1/src/ppc/assembler-ppc-inl.h
File src/ppc/assembler-ppc-inl.h (right):
https://codereview.chromium.org/1030353003/diff/1/src/ppc/assembler-ppc-inl.h#newcode574
src/ppc/assembler-ppc-inl.h:574: Memory::Address_at(pc) = target;
On 2015/04/08 12:38:55, rmcilroy (OOO till 18th May) wrote:
What is this branch for? It looks like pc is pointing to the constant
pool entry
- we shouldn't be passing this into set_target_address_at.
I was experimenting at one point with associating relocations with the
constant pool entry itself rather than the instruction loading the
entry. I will remove.
https://codereview.chromium.org/1030353003/diff/1/src/ppc/macro-assembler-ppc.cc
File src/ppc/macro-assembler-ppc.cc (right):
https://codereview.chromium.org/1030353003/diff/1/src/ppc/macro-assembler-ppc.cc#newcode686
src/ppc/macro-assembler-ppc.cc:686: } else {
On 2015/04/08 12:38:55, rmcilroy (OOO till 18th May) wrote:
I think these should be two separate functions (with and without the
base
regisiter)
Acknowledged.
https://codereview.chromium.org/1030353003/diff/1/src/ppc/macro-assembler-ppc.cc#newcode700
src/ppc/macro-assembler-ppc.cc:700:
LoadOwnConstantPoolPointerRegister(ip, -prologue_offset);
On 2015/04/08 12:38:55, rmcilroy (OOO till 18th May) wrote:
Where does ip get set to the prologue address?
All calls to JS code are done via the ip register. Any instructions
prior to the prologue are adjusted for -- see
LCodeGen::GeneratePrologue.
https://codereview.chromium.org/1030353003/diff/1/src/ppc/macro-assembler-ppc.cc#newcode737
src/ppc/macro-assembler-ppc.cc:737:
LoadOwnConstantPoolPointerRegister(ip, -prologue_offset);
On 2015/04/08 12:38:55, rmcilroy (OOO till 18th May) wrote:
ditto
see above.
https://codereview.chromium.org/1030353003/
--
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
---
You received this message because you are subscribed to the Google Groups "v8-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
For more options, visit https://groups.google.com/d/optout.
