Please make sure that all tests work in the intended way by verifying the
HIR
output.
I really think we should have some automated way of testing individual
optimizations. Otherwise, the effort of manually verifying that a test does
the
right thing is just enormous.
http://codereview.chromium.org/9141016/diff/29002/test/mjsunit/elements-transition-hoisting.js
File test/mjsunit/elements-transition-hoisting.js (right):
http://codereview.chromium.org/9141016/diff/29002/test/mjsunit/elements-transition-hoisting.js#newcode45
test/mjsunit/elements-transition-hoisting.js:45: // Make sure that
multiple stores to an object array get hoisted in a way that
What is meant by hoisting multiple stores? Stores themselves never get
hoisted.
http://codereview.chromium.org/9141016/diff/29002/test/mjsunit/elements-transition-hoisting.js#newcode52
test/mjsunit/elements-transition-hoisting.js:52: } while (global_false);
Maybe you should go around the loop >1 times in all tests where you want
to test that hoisting does not cause deopts.
http://codereview.chromium.org/9141016/diff/29002/test/mjsunit/elements-transition-hoisting.js#newcode101
test/mjsunit/elements-transition-hoisting.js:101: // Make sure that
non-element related map checks still get hoisted if all
The description of this test does not match the comment below where you
write that the map check should not be hoisted.
http://codereview.chromium.org/9141016/diff/29002/test/mjsunit/elements-transition-hoisting.js#newcode111
test/mjsunit/elements-transition-hoisting.js:111: a.foo = object; //
This map check should NOT be hoistable because includes
When running this test I see this map check hoisted which contradicts
your expectation here.
http://codereview.chromium.org/9141016/diff/29002/test/mjsunit/elements-transition-hoisting.js#newcode128
test/mjsunit/elements-transition-hoisting.js:128: a.foo = 0;
Since a.foo is not used in the loop, I don't see why it's necessary to
add it here.
http://codereview.chromium.org/9141016/diff/29002/test/mjsunit/elements-transition-hoisting.js#newcode131
test/mjsunit/elements-transition-hoisting.js:131: if (global_false !=
true) {
if (!global_false)
Otherwise the compare is generic (and a call inside the loop).
http://codereview.chromium.org/9141016/diff/29002/test/mjsunit/elements-transition-hoisting.js#newcode146
test/mjsunit/elements-transition-hoisting.js:146: a.foo = 0;
Since a.foo is not used in the loop, I don't see why it's necessary to
add it here.
http://codereview.chromium.org/9141016/
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
