https://chromiumcodereview.appspot.com/22935005/diff/1/src/x64/full-codegen-x64.cc
File src/x64/full-codegen-x64.cc (right):
https://chromiumcodereview.appspot.com/22935005/diff/1/src/x64/full-codegen-x64.cc#newcode4425
src/x64/full-codegen-x64.cc:4425: if (expr->op() == Token::INC) {
On 2013/08/20 14:36:05, haitao.feng wrote:
Thanks for the review and recommendation. I will make this CL an
incremental
step of introducing the SmiWrapper.
How about the other part of this CL? i.e., guiding the SmiAddConstant
and
SmiSubConstant by JumpIfNotSmi? Do we need to address the issue of
using
SmiAddConstant for both SMI and HeapNumber?
I like your current approach where the Smi/no Smi decision is made very
early, that looks good. The "heap number optimization" in the old code
was (as you point out) subtle and it's unclear to me that the old
version is actually a performance win. It saved on code size, but
without concrete performance numbers to show that it was better your
new, explicit split of smi/heap code number early-on seems c cleaner and
easier to understand.
I think there might even be a bug in the old code that you have fixed!
Previously, if there was an overflow in the smi constant add/sub case,
we'd jump to the stub... but not restore rax to it's pre-increment
state. The stub would work on the post-incremented value, not the
pre-incremented value!
https://chromiumcodereview.appspot.com/22935005/
--
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
---
You received this message because you are subscribed to the Google Groups "v8-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
For more options, visit https://groups.google.com/groups/opt_out.
