LGTM
http://codereview.chromium.org/2586001/diff/1/4 File src/ia32/builtins-ia32.cc (right): http://codereview.chromium.org/2586001/diff/1/4#newcode616 src/ia32/builtins-ia32.cc:616: __ CmpObjectType(ebx, FIRST_JS_OBJECT_TYPE, ecx); This seems to be comparing the instance type with a range, and not using the value after that. We should be able to reduce that to a single unsigned comparison and jump. Could we add a macro that did range-comparison, e.g., CmpObjectTypeRange(ebx, FIRST_JS_OBJECT_TYPE, LAST_JS_OBJECT_TYPE, ecx) where ecx is used as scratch (no guarantees on the value afterwards). http://codereview.chromium.org/2586001/diff/1/5 File src/ia32/codegen-ia32.cc (right): http://codereview.chromium.org/2586001/diff/1/5#newcode2735 src/ia32/codegen-ia32.cc:2735: // If the length is 0 then the subtraction gave -1, setting Comment is out of sync (there is no longer a subtraction). http://codereview.chromium.org/2586001/diff/1/5#newcode6311 src/ia32/codegen-ia32.cc:6311: __ CmpInstanceType(map.reg(), LAST_JS_OBJECT_TYPE); Again a range comparison. http://codereview.chromium.org/2586001/diff/1/5#newcode8325 src/ia32/codegen-ia32.cc:8325: __ CmpInstanceType(map.reg(), LAST_JS_OBJECT_TYPE); And again a range test. I think we use it often enough that it would be worth it do make a special macro for it. http://codereview.chromium.org/2586001/diff/1/5#newcode12849 src/ia32/codegen-ia32.cc:12849: ASSERT(kStringEncodingMask == kAsciiStringTag); Also assert that they are zero. Perhaps use STATIC_ASSERT, the variables should be compile time constants. http://codereview.chromium.org/2586001/show -- v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev
