Looks great. One suggestion for replacing the contents of a test.
A number of explicit optimizations of something that should be inlined. I think
they can be removed. http://codereview.chromium.org/6821009/diff/1/test/mjsunit/compiler/optimized-function-calls.js File test/mjsunit/compiler/optimized-function-calls.js (right): http://codereview.chromium.org/6821009/diff/1/test/mjsunit/compiler/optimized-function-calls.js#newcode52 test/mjsunit/compiler/optimized-function-calls.js:52: %OptimizeFunctionOnNextCall(object.f); I'm not sure you should do this. If you optimize call_f, does o.f get inlined? --trace-inlining will give you that info. http://codereview.chromium.org/6821009/diff/1/test/mjsunit/compiler/property-refs.js File test/mjsunit/compiler/property-refs.js (right): http://codereview.chromium.org/6821009/diff/1/test/mjsunit/compiler/property-refs.js#newcode51 test/mjsunit/compiler/property-refs.js:51: %OptimizeFunctionOnNextCall(StoreXY); Similarly here. If you do not optimize this here, does StoreXY get inlined in LoadXY? http://codereview.chromium.org/6821009/diff/1/test/mjsunit/compiler/recursive-deopt.js File test/mjsunit/compiler/recursive-deopt.js (right): http://codereview.chromium.org/6821009/diff/1/test/mjsunit/compiler/recursive-deopt.js#newcode39 test/mjsunit/compiler/recursive-deopt.js:39: assertEquals(1 << 1, f(0)); I think we need to completely change this test. This will always deopt in the base case here. I think we should get rid of RunTests all together. Use f(4) as our example. Call f(4) 5 times checking the result. Then optimize and call f(4) checking the result. Then replace 'var one' and then just call f(4) again checking the result. That will cause deopts at all the deopt places in the function. http://codereview.chromium.org/6821009/diff/1/test/mjsunit/compiler/regress-loadfield.js File test/mjsunit/compiler/regress-loadfield.js (right): http://codereview.chromium.org/6821009/diff/1/test/mjsunit/compiler/regress-loadfield.js#newcode66 test/mjsunit/compiler/regress-loadfield.js:66: %OptimizeFunctionOnNextCall(bar); Shouldn't be necessary to optimize bar here. It should be inlined into test I think. http://codereview.chromium.org/6821009/diff/1/test/mjsunit/compiler/regress-max.js File test/mjsunit/compiler/regress-max.js (right): http://codereview.chromium.org/6821009/diff/1/test/mjsunit/compiler/regress-max.js#newcode34 test/mjsunit/compiler/regress-max.js:34: %OptimizeFunctionOnNextCall(Math.max); This should get inlined if you just optimize f I think. http://codereview.chromium.org/6821009/diff/1/test/mjsunit/compiler/regress-stacktrace-methods.js File test/mjsunit/compiler/regress-stacktrace-methods.js (right): http://codereview.chromium.org/6821009/diff/1/test/mjsunit/compiler/regress-stacktrace-methods.js#newcode48 test/mjsunit/compiler/regress-stacktrace-methods.js:48: %OptimizeFunctionOnNextCall(Hest.prototype.three); Same here. http://codereview.chromium.org/6821009/diff/1/test/mjsunit/compiler/switch-bailout.js File test/mjsunit/compiler/switch-bailout.js (right): http://codereview.chromium.org/6821009/diff/1/test/mjsunit/compiler/switch-bailout.js#newcode40 test/mjsunit/compiler/switch-bailout.js:40: // [disabled optimization for: f / 7fb0972a2119] ?! Remove? http://codereview.chromium.org/6821009/diff/1/test/mjsunit/regress/regress-1106.js File test/mjsunit/regress/regress-1106.js (right): http://codereview.chromium.org/6821009/diff/1/test/mjsunit/regress/regress-1106.js#newcode56 test/mjsunit/regress/regress-1106.js:56: %OptimizeFunctionOnNextCall(x.gee); Shouldn't be necessary. Should be inlined. http://codereview.chromium.org/6821009/diff/1/test/mjsunit/regress/regress-1229.js File test/mjsunit/regress/regress-1229.js (right): http://codereview.chromium.org/6821009/diff/1/test/mjsunit/regress/regress-1229.js#newcode61 test/mjsunit/regress/regress-1229.js:61: %OptimizeFunctionOnNextCall(foo); Shouldn't be necessary. http://codereview.chromium.org/6821009/diff/1/test/mjsunit/regress/regress-1229.js#newcode88 test/mjsunit/regress/regress-1229.js:88: %OptimizeFunctionOnNextCall(baz); Shouldn't be needed. http://codereview.chromium.org/6821009/diff/1/test/mjsunit/regress/regress-1237.js File test/mjsunit/regress/regress-1237.js (right): http://codereview.chromium.org/6821009/diff/1/test/mjsunit/regress/regress-1237.js#newcode39 test/mjsunit/regress/regress-1237.js:39: %OptimizeFunctionOnNextCall(observe); Ditto. http://codereview.chromium.org/6821009/diff/1/test/mjsunit/regress/regress-962.js File test/mjsunit/regress/regress-962.js (right): http://codereview.chromium.org/6821009/diff/1/test/mjsunit/regress/regress-962.js#newcode57 test/mjsunit/regress/regress-962.js:57: %OptimizeFunctionOnNextCall(L.prototype.c); Ditto. http://codereview.chromium.org/6821009/ -- v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev
