Fixed register allocation corner case.

Fixed register allocation corner case.

The predicate CanBeSpilled had a bug, prohibiting the necessary spilling and
correct splitting of live ranges. Removed a redundant assertion immediately done
by the callee anyway.

Thanks to Slava for help with that issue and the entertaining historical
background of the whole story... ;-)

BUG=177883
Committed: http://code.google.com/p/v8/source/detail?r=13891

[Sven Panne, 2013-03-11 08:03:23]

Added regression test.

[danno, 2013-03-11 08:39:57]

lgtm with comments

https://codereview.chromium.org/12631012/diff/3001/test/mjsunit/compiler/regr...
File test/mjsunit/compiler/regress-177883.js (right):

https://codereview.chromium.org/12631012/diff/3001/test/mjsunit/compiler/regr...
test/mjsunit/compiler/regress-177883.js:48: return +(+HEAPF32[i1 >> 2] *
+HEAPF32[i2 >> 2] + +HEAPF32[i1 + 4 >> 2] * +HEAPF32[i2 + 4 >> 2] + +HEAPF32[i1
+ 8 >> 2] * +HEAPF32[i2 + 8 >> 2] + +HEAPF32[i1 + 12 >> 2] * +HEAPF32[i2 + 12 >>
2]);
nits: 80 col here and below

[Vyacheslav Egorov (Chromium), 2013-03-11 08:43:15]

LGTM

https://codereview.chromium.org/12631012/diff/3001/src/lithium-allocator.cc
File src/lithium-allocator.cc (left):

https://codereview.chromium.org/12631012/diff/3001/src/lithium-allocator.cc#o...
src/lithium-allocator.cc:1925: ASSERT(current->Start().Value() <
register_use->pos().Value());
I wonder if we should turn ASSERT in the callee into the CHECK to prevent hangs
and get crashes instead.

https://codereview.chromium.org/12631012/diff/3001/test/mjsunit/compiler/regr...
File test/mjsunit/compiler/regress-177883.js (right):

https://codereview.chromium.org/12631012/diff/3001/test/mjsunit/compiler/regr...
test/mjsunit/compiler/regress-177883.js:42: // allocator tried to split of an
empty range, making no progress at all.
register allocator was splitting live range at its start readding the very same
range into the list of unallocated ranges and thus making no progress.

[Sven Panne, 2013-03-11 09:48:03]

https://codereview.chromium.org/12631012/diff/3001/src/lithium-allocator.cc
File src/lithium-allocator.cc (left):

https://codereview.chromium.org/12631012/diff/3001/src/lithium-allocator.cc#o...
src/lithium-allocator.cc:1925: ASSERT(current->Start().Value() <
register_use->pos().Value());
On 2013/03/11 08:43:15, Vyacheslav Egorov wrote:
> I wonder if we should turn ASSERT in the callee into the CHECK to prevent
hangs
> and get crashes instead.

Sounds like a good idea, the check is extremely cheap. I'll change SpillBetween
accordingly.

https://codereview.chromium.org/12631012/diff/3001/test/mjsunit/compiler/regr...
File test/mjsunit/compiler/regress-177883.js (right):

https://codereview.chromium.org/12631012/diff/3001/test/mjsunit/compiler/regr...
test/mjsunit/compiler/regress-177883.js:42: // allocator tried to split of an
empty range, making no progress at all.
On 2013/03/11 08:43:15, Vyacheslav Egorov wrote:
> register allocator was splitting live range at its start readding the very
same
> range into the list of unallocated ranges and thus making no progress.

Done.

https://codereview.chromium.org/12631012/diff/3001/test/mjsunit/compiler/regr...
test/mjsunit/compiler/regress-177883.js:48: return +(+HEAPF32[i1 >> 2] *
+HEAPF32[i2 >> 2] + +HEAPF32[i1 + 4 >> 2] * +HEAPF32[i2 + 4 >> 2] + +HEAPF32[i1
+ 8 >> 2] * +HEAPF32[i2 + 8 >> 2] + +HEAPF32[i1 + 12 >> 2] * +HEAPF32[i2 + 12 >>
2]);
On 2013/03/11 08:39:57, danno wrote:
> nits: 80 col here and below

As discussed offline, let's keep this as it is if our tools are happy with it.
Furthermore, actually splitting the lines hurts readability far more than
helping it. The Google style guide is basically useless with mangled C++
names...

[Sven Panne, 2013-03-11 09:49:06]

Committed patchset #3 manually as r13891 (presubmit successful).

[Sven Panne, 2013-03-11 10:40:05]

FYI: The test case is already not a test case anymore, my guess is that
https://code.google.com/p/v8/source/detail?r=13890 changed enough to make the
corner case disappear for the code at hand. We knew it was brittle, but that was
*fast*... :-[