[turbofan] reduce ResolveControlFlow overhead.

[turbofan] reduce ResolveControlFlow overhead.

I found this optimization opportunity when analyzing some pathological
compile-time examples. When tying together live ranges across control
flow boundaries, we used to repeatedly check if the top level range was
spilled in deferred blocks or not. This proved to be a hotspot in such
cases (i.e. the pathological compile time ones).

Because the analysis needs to progress block by block and not live range by live range, we cannot feasibly save per-range information to
remove the hotspot. Instead, we save this information when
constructing LiveRangeBounds.

The result is 2.5 to 7% improvement in the pathological cases, and a few
similar bonuses in perf in a couple of other benchmarks.

Also, opportunistically removed the loop counting the number of child
ranges, since we have that count from the new (post - refactoring)
range numbering technique.

BUG=

Committed: https://crrev.com/0bcfb26e7c45664ed94b1139eeb3f580725929fd
Cr-Commit-Position: refs/heads/master@{#32071}

[Mircea Trofin, 2015-11-16 20:36:00]

Patchset #1 (id:1) has been deleted

[Mircea Trofin, 2015-11-17 01:04:00]

Patchset #2 (id:40001) has been deleted

[Mircea Trofin, 2015-11-17 01:04:18]

Patchset #1 (id:20001) has been deleted

[Mircea Trofin, 2015-11-17 05:03:58]

Description was changed from

==========
[turbofan] reduce ResolveControlFlow overhead.

BUG=
==========

to

==========
[turbofan] reduce ResolveControlFlow overhead.

I found this optimization opportunity when analyzing some pathological
compile-time examples. When tying together live ranges across control
flow boundaries, we used to repeatedly check if the top level range was 
spilled in deferred blocks or not. This proved to be a hotspot in such 
cases (i.e. the pathological compile time ones). 

Because the analysis needs to progress block by block and not live range by live
range, we cannot feasibly save per-range information to 
remove the hotspot. Instead, we save this information when
constructing LiveRangeBounds.

The result is 2.5 to 7% improvement in the pathological cases, and a few
similar bonuses in perf in a couple of other benchmarks.

Also, opportunistically removed the loop counting the number of child
ranges, since we have that count from the new (post - refactoring) 
range numbering technique.

BUG=
==========

[Mircea Trofin, 2015-11-17 05:03:58]

mtrofin@chromium.org changed reviewers:
+ bmeurer@chromium.org, jarin@chromium.org

[Jarin, 2015-11-17 14:53:12]

https://codereview.chromium.org/1412573009/diff/60001/src/compiler/register-a...
File src/compiler/register-allocator.cc (right):

https://codereview.chromium.org/1412573009/diff/60001/src/compiler/register-a...
src/compiler/register-allocator.cc:3215: new (curr) LiveRangeBound(i,
!spilled_in_blocks && i->spilled());
Could you add a comment here explaining why we can skip the connecting move if
the (sub-)range is spilled and there are spills outside deferred blocks. (I
suppose it is because the connecting move would move into a spill slot, but that
should be already taken care of at definition.)

https://codereview.chromium.org/1412573009/diff/60001/src/compiler/register-a...
src/compiler/register-allocator.cc:3260: if (result->pred_cover_ !=
bound->range_) {
I am confused about this condition - on line 3255, we have just assigned
result->pred_cover_ = bound->range_, so I do not see how this can be ever taken.

[Mircea Trofin, 2015-11-17 16:57:05]

https://codereview.chromium.org/1412573009/diff/60001/src/compiler/register-a...
File src/compiler/register-allocator.cc (right):

https://codereview.chromium.org/1412573009/diff/60001/src/compiler/register-a...
src/compiler/register-allocator.cc:3215: new (curr) LiveRangeBound(i,
!spilled_in_blocks && i->spilled());
On 2015/11/17 14:53:12, Jarin wrote:
> Could you add a comment here explaining why we can skip the connecting move if
> the (sub-)range is spilled and there are spills outside deferred blocks. (I
> suppose it is because the connecting move would move into a spill slot, but
that
> should be already taken care of at definition.)

Done.

https://codereview.chromium.org/1412573009/diff/60001/src/compiler/register-a...
src/compiler/register-allocator.cc:3260: if (result->pred_cover_ !=
bound->range_) {
On 2015/11/17 14:53:12, Jarin wrote:
> I am confused about this condition - on line 3255, we have just assigned
> result->pred_cover_ = bound->range_, so I do not see how this can be ever
taken.

Thanks for the catch - the test was redundant. Added a comment to explain a bit
what's happening, too.

[Jarin, 2015-11-18 06:26:37]

lgtm. thanks!

[Mircea Trofin, 2015-11-18 06:26:53]

The CQ bit was checked by mtrofin@chromium.org

[commit-bot: I haz the power, 2015-11-18 06:27:02]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1412573009/80001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1412573009/80001

[commit-bot: I haz the power, 2015-11-18 06:47:52]

Committed patchset #2 (id:80001)

[commit-bot: I haz the power, 2015-11-18 06:48:16]

Patchset 2 (id:??) landed as
https://crrev.com/0bcfb26e7c45664ed94b1139eeb3f580725929fd
Cr-Commit-Position: refs/heads/master@{#32071}