Looks good, but I think you could go a little further with these
functions. See
comments. below.
http://codereview.chromium.org/1732024/diff/1/2
File src/arm/codegen-arm.cc (right):
http://codereview.chromium.org/1732024/diff/1/2#newcode1566
src/arm/codegen-arm.cc:1566: frame_->EmitPush(cp);
Since we have to spill all registers to the stack to do the runtime call
this part of the change won't have much effect. At best it may
substitute an stm for two pushes if we are lucky.
http://codereview.chromium.org/1732024/diff/1/2#newcode1573
src/arm/codegen-arm.cc:1573: {
The braces here are not necessary.
http://codereview.chromium.org/1732024/diff/1/2#newcode1599
src/arm/codegen-arm.cc:1599: Register reg = frame_->GetTOSRegister();
We should have an EmitPush(Operand op) as a shorthand for these three.
It would be almost identical in implementation to EmitPush(MemOperand).
http://codereview.chromium.org/1732024/diff/1/2#newcode1617
src/arm/codegen-arm.cc:1617: VirtualFrame::SpilledScope
spilled_scope(frame_);
No need for this spilled scope really. Just substitute Load for
LoadAndSpill in the line immediately below.
http://codereview.chromium.org/1732024/diff/1/2#newcode1624
src/arm/codegen-arm.cc:1624: {
No need for these braces.
http://codereview.chromium.org/1732024/diff/1/2#newcode1645
src/arm/codegen-arm.cc:1645: VirtualFrame::SpilledScope
spilled_scope(frame_);
I don't think you need this spilled scope. The LoadAndSpill can be a
Load and the target.SetValue is able to cope with non-spilled frames.
There are no JumpTargets or deferred code here that would get confused
about non-spilled frames. Drop can cope with and take advantage of a
non-spilled frame too.
http://codereview.chromium.org/1732024/show
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
