This doesn't look quite right.

The load and store ICs expect values passed on the stack that are not
consumed by the ICs.  We need to ensure that those values are in memory
for the IC code.


http://codereview.chromium.org/18596/diff/1/2
File src/codegen-ia32.cc (right):

http://codereview.chromium.org/18596/diff/1/2#newcode4607
Line 4607: // They are spilled in CallCodeObject.
As discussed, they are not spilled by CallCodeObject.  The simplest
thing is to SpillAll before the CallCodeObject.

Ultimately we will want something better.

http://codereview.chromium.org/18596/diff/1/2#newcode4611
Line 4611: RelocInfo::CODE_TARGET_CONTEXT, 0);
Put the zero on the next line, lest it get lost.

http://codereview.chromium.org/18596/diff/1/2#newcode4619
Line 4619: // the push that follows might be peep-hole optimized away.
Change the comment about peephole optimization to say something like the
push to the virtual frame will be performed lazily only when needed.

http://codereview.chromium.org/18596/diff/1/2#newcode4621
Line 4621: frame->Push(&answer);  // IC call leaves result in eax, push
it out
Just remove this comment.

http://codereview.chromium.org/18596/diff/1/2#newcode4681
Line 4681: Result argument = frame->Pop();
Not really "argument".  It is the value being stored.

http://codereview.chromium.org/18596/diff/1/2#newcode4684
Line 4684: Result ecx_result = cgen_->allocator()->Allocate(ecx);
I think name_register or something is a better name than ecx_result.

http://codereview.chromium.org/18596/diff/1/2#newcode4689
Line 4689: argument.Unuse();
As discussed, we should probably pass these registers to CallCode object
to be consumed, rather than explicitly unusing them.

http://codereview.chromium.org/18596/diff/1/2#newcode4691
Line 4691: Result answer = frame->CallCodeObject(ic,
RelocInfo::CODE_TARGET, 0);
I think this needs some sort of spill solution.  The receiver is on the
stack, used by the call, and will not necessarily be spilled.

http://codereview.chromium.org/18596/diff/1/2#newcode4701
Line 4701: Result temp = frame->Pop();
Not really a temp, it is the value being stored.

http://codereview.chromium.org/18596/diff/1/2#newcode4704
Line 4704: temp.Unuse();
Ditto about having CallCodeObject consume the register reference rather
than explicitly unusing it.

http://codereview.chromium.org/18596/diff/1/2#newcode4705
Line 4705: temp = frame->CallCodeObject(ic, RelocInfo::CODE_TARGET, 0);
Same comment about spilling.  The key and receiver are both on the stack
and used by the call, but not necessarily spilled.

http://codereview.chromium.org/18596

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

Reply via email to