Thanks serya for your comments.

http://codereview.chromium.org/6170001/diff/60001/src/arm/macro-assembler-arm.cc
File src/arm/macro-assembler-arm.cc (right):

http://codereview.chromium.org/6170001/diff/60001/src/arm/macro-assembler-arm.cc#newcode1464
src/arm/macro-assembler-arm.cc:1464: int frame_alignment =
ActivationFrameAlignment();
On 2011/01/20 12:09:28, SeRya wrote:
Currently CEntryStub stack layout is following:
<caller stack frame>
<stack alignment place holder initialized by 0?>
<caller frame pointer>
<caller stack pointer after unwinding>
<return address>
<code object>
<return address to the stub or 0-marker>
<fp state?>

Your layout:
<caller stack frame>
<caller frame pointer>
<caller stack pointer after unwinding>
<1-marker>
<pointer to the return address>
<stack alignment uninitialized place holder?>
<return address>
<code object>
<return address>
Explaining the issue below, my layout in bit more detail
<caller stack frame>
<return address>
<caller stack pointer after unwinding>
<caller frame pointer>
<code object> <-- centry sp, cant use the same in current case
<marker>
<exit frame sp>
.
.
<arguments>
.
.
<stack alignment>
<return address>
<native call stack>

I cannot completely reuse the exit frame layout since arguments come in
between exit frame and the native entry and hence the need to put
additional slot (marker/stack) to point to return address.

If you already considered the above issue, maybe i miss your point.


You can just move the stack alignment place holder above,
 initialize it by 0,
move the maker and pointer to the return address and your layout will
be fully
compatible with CEntryStub.
I think i can reuse EnterExitFrame by moving the code you mentioned
below and also making frame alignment code generic.

You won't need a special case in
ExitFrame::ComputeStackPointer and ExitApiFrameConstants.
From above comments iam not sure how to avoid this.

Actually I think you should MacroAssembler::EnterExitFrame. Just move
the
following lines out of the methods and it will do exactly what do you
need here:

   add(ip, sp, Operand(r0, LSL, kPointerSizeLog2));
   sub(r6, ip, Operand(kPointerSize));
...
  // Setup argc and the builtin function in callee-saved registers.
   mov(r4, Operand(r0));
   mov(r5, Operand(r1));
you mean i should move these back to CEntryStub::Generate and introduce
a parameter (say pending_push) which is used to do frame alignment.

http://codereview.chromium.org/6170001/diff/60001/src/arm/macro-assembler-arm.cc#newcode1480
src/arm/macro-assembler-arm.cc:1480: str(sp, MemOperand(fp,
ExitApiFrameConstants::kSPOffset));
On 2011/01/20 12:09:28, SeRya wrote:
It looks like this sequence is equal to the shorter one:

if (frame_alignment > kPointerSize) {
   ASSERT(frame_alignment == 2 * kPointerSize);
   tst(sp, Operand(frame_alignment_mask));
   // Stack alignment place holder need not be initialized as its below
   // c_entry_fp_address and does not affect GC.
   push(ip, eq);
}

// Store sp in the exit frame sp slot. sp - 1 points to return address
// pushed before call
str(sp, MemOperand(fp, ExitApiFrameConstants::kSPOffset));

(removed sub/add instructions and condition in the push instruction is
changed
to the opposite one).
Thanks for the catch.

http://codereview.chromium.org/6170001/

--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev

Reply via email to