LGTM. The new encoding is quite clever and it could use a comment in objects.h.
Let's hope we don't have to change the encoding much. http://codereview.chromium.org/8187/diff/1/12 File src/codegen-ia32.cc (right): http://codereview.chromium.org/8187/diff/1/12#newcode2868 Line 2868: __ shr(edx); // Ecx is implicit operand. Since Ecx is a register name, I probably would not capitalize it. http://codereview.chromium.org/8187/diff/1/8 File src/objects.h (right): http://codereview.chromium.org/8187/diff/1/8#newcode398 Line 398: // If bit 7 is clear, bits 5 and 6 are the string's size (short, medium or long). This is now bits 4 and 5? Also, you should add a comment explaining that you use this encoding so that you can perform the length shifts based on the tags in the instance type. http://codereview.chromium.org/8187/diff/1/8#newcode404 Line 404: // If bit 7 is clear, bit 4 indicates that the string is a symbol (if set) or And this is now bit 5? http://codereview.chromium.org/8187/diff/1/8#newcode417 Line 417: // If bit 7 is clear, the low-order 3 bits indicate the representation 3 -> 2 bits. http://codereview.chromium.org/8187/diff/1/8#newcode3157 Line 3157: static const int kMaxMediumStringSize = 16383; These reductions in the limits is the only thing that worries me. It means that a lot of strings could potentially have worse hash-codes. http://codereview.chromium.org/8187/diff/1/8#newcode3183 Line 3183: static const int kShortLengthShift = 26; Could we write these as kShortStringTag + kRepresentationTagSize and kMediumStringTag + kRepresentationTagSize. That would make it clearer to me what the encoding is. http://codereview.chromium.org/8187/diff/1/2 File src/stub-cache-ia32.cc (right): http://codereview.chromium.org/8187/diff/1/2#newcode182 Line 182: // Ecx is also the receiver. Ecx -> ecx since it is a register name? http://codereview.chromium.org/8187/diff/1/2#newcode184 Line 184: __ shr(eax); // Ecx is implicit shift register. Ditto? http://codereview.chromium.org/8187 --~--~---------~--~----~------------~-------~--~----~ v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev -~----------~----~----~----~------~----~------~--~---
