The patch looks good, but the regression test doesn't seem to trigger the
wrong
behavior.
http://codereview.chromium.org/10802038/diff/6/test/mjsunit/regress/regress-137768.js
File test/mjsunit/regress/regress-137768.js (right):
http://codereview.chromium.org/10802038/diff/6/test/mjsunit/regress/regress-137768.js#newcode35
test/mjsunit/regress/regress-137768.js:35: s += Math.floor(a);
Did you mean floor(x)? Either way, this test works just fine for me with
bleeding_edge r12151, it doesn't crash and the load is not hoisted out
of the loop (interestingly, with --packed-arrays it's a generic load).
Am I missing something?
http://codereview.chromium.org/10802038/diff/6/test/mjsunit/regress/regress-137768.js#newcode50
test/mjsunit/regress/regress-137768.js:50: // Optimize, bofre the fix,
the element load and subsequent tagged-to-i were
nit: messed up comment ("bofre", incomplete sentence).
http://codereview.chromium.org/10802038/diff/6/test/mjsunit/regress/regress-137768.js#newcode59
test/mjsunit/regress/regress-137768.js:59: // tagged-to-i will treat
part of a double value as a pointer and de-ref the
nit: slightly messed up comment. s/de-ref the/de-ref it/ ?
http://codereview.chromium.org/10802038/
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev