Fix elements-kind test to disable optimization of important functions under test; add simpler versi…

Fix elements-kind test to disable optimization of important functions under test; add simpler versions of elements kind test.

Committed: https://code.google.com/p/v8/source/detail?r=15347

[mvstanton, 2013-06-26 13:21:43]

Hi Ben, just a cupla questions.
--Michael

https://codereview.chromium.org/17872002/diff/1/test/mjsunit/elements-kind.js
File test/mjsunit/elements-kind.js (right):

https://codereview.chromium.org/17872002/diff/1/test/mjsunit/elements-kind.js...
test/mjsunit/elements-kind.js:226: try {} catch (e) {} // TODO(titzer):
DisableOptimization
What is the timeframe to bring these back to allow optimization, I ask because
it looks like the intent of this test is to verify that things work as expected
in the full codegen and crankshaft cases. If we don't test the crankshaft case,
regressions could slip in. Is there another way to achieve what you are looking
for, like making a copy of this test and specifying different flags?

https://codereview.chromium.org/17872002/diff/1/test/mjsunit/opt-elements-kin...
File test/mjsunit/opt-elements-kind.js (right):

https://codereview.chromium.org/17872002/diff/1/test/mjsunit/opt-elements-kin...
test/mjsunit/opt-elements-kind.js:29: // Flags: --notrack_allocation_sites
In these new tests, why do you have to turn off tracking allocation sites?

[titzer, 2013-06-26 17:53:59]

https://codereview.chromium.org/17872002/diff/1/test/mjsunit/elements-kind.js
File test/mjsunit/elements-kind.js (right):

https://codereview.chromium.org/17872002/diff/1/test/mjsunit/elements-kind.js...
test/mjsunit/elements-kind.js:226: try {} catch (e) {} // TODO(titzer):
DisableOptimization
On 2013/06/26 13:21:43, mvstanton wrote:
> What is the timeframe to bring these back to allow optimization, I ask because
> it looks like the intent of this test is to verify that things work as
expected
> in the full codegen and crankshaft cases. If we don't test the crankshaft
case,
> regressions could slip in. Is there another way to achieve what you are
looking
> for, like making a copy of this test and specifying different flags?

As it turns out, what this test does is check the elements kind transitions that
happen in fullcode; i.e. it will not work if the methods in question are
compiled with crankshaft, which transitions the elements kinds differently. As
it turns out, these methods were never being compiled with Crankshaft, and when
I added a mode to do aggressive OSR (including the toplevel code here, which
then inlines these methods), then one of this test's asserts fails.

I'm not a big fan of tests for optimization states, in particular this one, but
at least this way, the tests' requirements are explicit. Before, we were just
lucky.

https://codereview.chromium.org/17872002/diff/1/test/mjsunit/opt-elements-kin...
File test/mjsunit/opt-elements-kind.js (right):

https://codereview.chromium.org/17872002/diff/1/test/mjsunit/opt-elements-kin...
test/mjsunit/opt-elements-kind.js:29: // Flags: --notrack_allocation_sites
On 2013/06/26 13:21:43, mvstanton wrote:
> In these new tests, why do you have to turn off tracking allocation sites?

It was turned off in the test from which I extracted this one (the one you
previously commented on). The copy detection is wrong about the origin of this
code.

[mvstanton, 2013-06-27 07:09:31]

Right on, sounds good. lgtm.

[titzer, 2013-06-27 08:46:53]

Committed patchset #1 manually as r15347 (presubmit successful).