http://codereview.chromium.org/3666001/diff/9001/10001
File src/arm/codegen-arm.cc (right):

http://codereview.chromium.org/3666001/diff/9001/10001#newcode6068
src/arm/codegen-arm.cc:6068: __ sub(value_, value_,
Operand(Smi::FromInt(1)), LeaveCC, vs);
The tst instruction does not affect the overflow flag, so for non-Smis,
which also branch here, it will be indeterminate whether or not the
optimistic increment/decrement (which didn't happen) is reverted.

http://codereview.chromium.org/3666001/diff/9001/10001#newcode6077
src/arm/codegen-arm.cc:6077: VirtualFrame::SpilledScope spilled(frame_);
This is not how it's normally done.  See
DeferredReferenceGetNamedValue::Generate to see how to handle the
virtual frame in a deferred ::Generate() method.

You should  save the frame with something like:

  VirtualFrame copied_frame(*frame_state()->frame());
  copied_frame.SpillAll();

This will get the frame state (the description of which registers
contain the top of stack) from the point where the DeferredCode subclass
was constructed.  You are expected not to change the virtual frame
between then and the point(s) where you branch to deferred code and the
point where you bind the exit of the deferred code.  You can then
restore it at the end of the deferred code with

  copied_frame.MergeTo(frame_state()->frame());

Within the ::Generate method you normally don't use EmitPush, just use
__ push() etc.

http://codereview.chromium.org/3666001/diff/9001/10001#newcode6197
src/arm/codegen-arm.cc:6197: exit.Branch(vc);
Ideally we would like to have the non-deferred case just fall through.
That means all the code that this branch skips should be moved into the
deferred code block.

http://codereview.chromium.org/3666001/diff/9001/10001#newcode6205
src/arm/codegen-arm.cc:6205:
frame_->UpdateFrameForDeferredCountOperation();
This forces the frame state to be flushed to stack (no registers contain
stack values), but it doesn't actually update the stack, just the
compiler state.  It seems wrong to me and I don't understand how it can
work, except by accident.  We are in a register allocation scope and the
top of stack could be in registers.

http://codereview.chromium.org/3666001/show

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

Reply via email to