On 2015/05/07 20:38:33, mtbrandyberry wrote:
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.
Resubmitted via new issue: https://codereview.chromium.org/1131783003/
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.
