One bug in the arm version. Other than that LGTM
http://codereview.chromium.org/6805005/diff/4009/src/arm/lithium-arm.cc File src/arm/lithium-arm.cc (right): http://codereview.chromium.org/6805005/diff/4009/src/arm/lithium-arm.cc#newcode1887 src/arm/lithium-arm.cc:1887: : UseRegister(instr->key()); key -> value. This should fail a test that Jakob added today. http://codereview.chromium.org/6805005/diff/4009/src/ast.h File src/ast.h (right): http://codereview.chromium.org/6805005/diff/4009/src/ast.h#newcode713 src/ast.h:713: int position() const { return position_; } Repeat the virtual keyword all the way down for virtual methods. http://codereview.chromium.org/6805005/diff/4009/src/hydrogen.cc File src/hydrogen.cc (right): http://codereview.chromium.org/6805005/diff/4009/src/hydrogen.cc#newcode3217 src/hydrogen.cc:3217: instr = BuildStoreKeyed(object, Looks like this will not fit on one line? http://codereview.chromium.org/6805005/diff/4009/src/hydrogen.cc#newcode3285 src/hydrogen.cc:3285: expr->RecordTypeFeedback(oracle()); Maybe move this to the only branch where it is needed? http://codereview.chromium.org/6805005/diff/4009/src/hydrogen.cc#newcode4587 src/hydrogen.cc:4587: expr->RecordTypeFeedback(oracle()); Move to where it is needed? http://codereview.chromium.org/6805005/diff/4009/src/hydrogen.h File src/hydrogen.h (right): http://codereview.chromium.org/6805005/diff/4009/src/hydrogen.h#newcode828 src/hydrogen.h:828: HInstruction* BuildLoadKeyed(HValue* obj, Remove spaces between '*' and 'obj'. Ditto for other occurrences. http://codereview.chromium.org/6805005/diff/4009/src/ia32/lithium-codegen-ia32.cc File src/ia32/lithium-codegen-ia32.cc (right): http://codereview.chromium.org/6805005/diff/4009/src/ia32/lithium-codegen-ia32.cc#newcode2314 src/ia32/lithium-codegen-ia32.cc:2314: default: Why do you want a default case here? Isn't it better to not have it so you realize when you add a new type? http://codereview.chromium.org/6805005/diff/4009/src/ia32/lithium-codegen-ia32.cc#newcode2984 src/ia32/lithium-codegen-ia32.cc:2984: default: Ditto. http://codereview.chromium.org/6805005/diff/4009/test/cctest/test-api.cc File test/cctest/test-api.cc (right): http://codereview.chromium.org/6805005/diff/4009/test/cctest/test-api.cc#newcode11527 test/cctest/test-api.cc:11527: " for (var i=0;i<40;++i) {" Add some spacing in all these loops? http://codereview.chromium.org/6805005/ -- v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev
