LGTM with comments.
http://codereview.chromium.org/160453/diff/1/7 File src/x64/assembler-x64.h (right): http://codereview.chromium.org/160453/diff/1/7#newcode56 Line 56: static inline bool is_int32(uint64_t x) { I can forsee a bug happening because of this function. I think we sometimes use a uint64_t for data that really includes negative values. The proper function for a person to use in that case is is_int32(int64_t x), but they would need a cast to use it. Could this be called uint_is_int32 to make it clear to the user that it should be used carefully? http://codereview.chromium.org/160453/diff/1/9 File src/x64/macro-assembler-x64.cc (right): http://codereview.chromium.org/160453/diff/1/9#newcode161 Line 161: j(zero, &done); Remove this smi test - we think it isn't worth it. http://codereview.chromium.org/160453/diff/1/9#newcode170 Line 170: } else { This optimization is unsafe if object can be in the upper half of the address space. Remove it. http://codereview.chromium.org/160453 --~--~---------~--~----~------------~-------~--~----~ v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev -~----------~----~----~----~------~----~------~--~---
