LGTM!

http://codereview.chromium.org/5714001/diff/9001/src/ia32/full-codegen-ia32.cc
File src/ia32/full-codegen-ia32.cc (right):

http://codereview.chromium.org/5714001/diff/9001/src/ia32/full-codegen-ia32.cc#newcode1557
src/ia32/full-codegen-ia32.cc:1557: masm_->bind(&patch_site_);
In debug mode in ~JumpPatchSite we could assert that patch info is
emitted if patch_site is bound.

http://codereview.chromium.org/5714001/diff/9001/src/ia32/full-codegen-ia32.cc#newcode1563
src/ia32/full-codegen-ia32.cc:1563: __ test(eax,
Immediate(delta_to_patch_site << 16 | cc_));
For smi checks the condition is always not_zero. We could save 3 bytes
by using short form of test here. Of course, this makes JumpPatchSite
less generic. On the other hand, we could specialize it even more for
smi checks, e.g.:
SmiCheckPatchSite smi_check(masm_, &call_stub);
smi_check.EmitCheck(eax);
...
CallStub(...);
smi_check.EmitPatchInfo();

http://codereview.chromium.org/5714001/diff/9001/src/ia32/full-codegen-ia32.cc#newcode1597
src/ia32/full-codegen-ia32.cc:1597: __ CallStub(&stub);
Consider adding a function like EmitCallIC that takes both a stub to
call and an optional patch site. Patch site should be used if it is
bound, otherwise nop is emitted.

http://codereview.chromium.org/5714001/

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

Reply via email to