http://codereview.chromium.org/1846002/diff/17001/18001 File src/arm/codegen-arm.cc (right):
http://codereview.chromium.org/1846002/diff/17001/18001#newcode3332 src/arm/codegen-arm.cc:3332: ASSERT_EQ(NULL, var); On 2010/05/03 19:16:50, Erik Corry wrote:
How do we know this assert is true? I would like more comments in
this function
explaining what the stack holds at each point. Perhaps the flags (is_trivial_receiver, starts_initialization_block,
ends_initialization_block,
is_compound, var->isnull) can be distilled into a set of orthogonal
variables
and the stack layout explained for each value of each variable.
Initialization block consists of assignments on the form expr.x = ..., so this will never be an assignment to a variable. I have tried to improve the comments and provide stack layout so that it is easier to follow what is going on. http://codereview.chromium.org/1846002/diff/17001/18001#newcode3361 src/arm/codegen-arm.cc:3361: Load(node->value()); On 2010/05/03 19:16:50, Erik Corry wrote:
If this is a constant Smi can we use a SmiOperation instead?
Yes we can. http://codereview.chromium.org/1846002/diff/17001/18001#newcode3377 src/arm/codegen-arm.cc:3377: frame_->EmitPop(VirtualFrame::scratch0()); On 2010/05/03 19:16:50, Erik Corry wrote:
Seems a bit dodgy assuming that the scratch registers don't get
overwritten by a
trivial receiver. If you are satisfied that this is currently the
case then
lets leave it in, but we need to start tracking off-stack registers
soon. Changed to swap [tos] with [tos+1] after loading the receiver. http://codereview.chromium.org/1846002/diff/17001/18001#newcode3406 src/arm/codegen-arm.cc:3406: void CodeGenerator::EmitKeyedPropertyAssignment(Assignment* node) { On 2010/05/03 19:16:50, Erik Corry wrote:
All the same comments apply to this function as the function above.
Done. http://codereview.chromium.org/1846002/diff/17001/18003 File src/arm/virtual-frame-arm.cc (right): http://codereview.chromium.org/1846002/diff/17001/18003#newcode513 src/arm/virtual-frame-arm.cc:513: __ mov(r1, r0); On 2010/05/03 19:16:50, Erik Corry wrote:
You forget to update the top_of_stack_state_ here and in the next
case. Thats right, but there is no need. r0 and r1 have the duplicated value, so both R0_R1_TOS and R1_R0_TOS are fine. Added a comment though. http://codereview.chromium.org/1846002/show -- v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev
