LGTM from my side. Kevin, do you have any more comments?
http://codereview.chromium.org/5753005/diff/17001/src/hydrogen-instructions.h File src/hydrogen-instructions.h (right): http://codereview.chromium.org/5753005/diff/17001/src/hydrogen-instructions.h#newcode2528 src/hydrogen-instructions.h:2528: SetFlag(kDependsOnCalls); The flags are ok now since we don't compile stores to context slots. But once we do, you need to add a new flag for stores and mark HLoadContextSlot depend on it, so the GVN does the right thing. http://codereview.chromium.org/5753005/diff/17001/test/mjsunit/closures.js File test/mjsunit/closures.js (right): http://codereview.chromium.org/5753005/diff/17001/test/mjsunit/closures.js#newcode29 test/mjsunit/closures.js:29: for (var i = 0; i < 100000000; i++) { 100M seems a bit high (at least for debug mode) - Could this count be lower to save time when running the tests and still hit the function with the optimizer? http://codereview.chromium.org/5753005/ -- v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev
