Ensure Schedules generated by the RawMachineAssembler are in edge-split form

Ensure Schedules generated by the RawMachineAssembler are in edge-split form

This CL adds an extra pass before calculating the special RPO
order in the custom RawMachineAssembler pipeline that
walks through the schedule and inserts extra blocks to
guarantee that the control flow graph is in split edge form. It
also propagates deferred block marks forward to these new
blocks if appropriate.

Committed: https://crrev.com/a827400025be77a75880b1255a269df52dca103d
Cr-Commit-Position: refs/heads/master@{#35014}

[danno, 2016-03-18 15:03:02]

Description was changed from

==========
Ensure split-edge form in CodeStubAssembler
==========

to

==========
Ensure Schedules generated by the RawMachineAssembler are in edge-split form

This CL adds an extra pass before calculating the special RPO
order in the custom RawMachineAssembler pipeline that
walks through the schedule and inserts extra blocks to 
guarantee that the control flow graph is in split edge form. It
also propagates deferred block marks forward to these new
blocks if appropriate.
==========

[danno, 2016-03-18 15:03:02]

danno@chromium.org changed reviewers:
+ mstarzinger@chromium.org, titzer@chromium.org

[danno, 2016-03-18 15:04:12]

PTAL. Note that even though I verified through another CL I'm working on, I will
add explicit tests before landing.

[danno, 2016-03-18 18:44:24]

Tests added, I think this is ready to go. Please take a look.

[titzer, 2016-03-21 08:06:03]

https://codereview.chromium.org/1811333002/diff/100001/src/compiler/instructi...
File src/compiler/instruction-selector.cc (right):

https://codereview.chromium.org/1811333002/diff/100001/src/compiler/instructi...
src/compiler/instruction-selector.cc:810: // The scheduler may have inserted a
split-edge block between the switch
Not sure this is sound. The split-edge blocks are the placement points for phi
resolution moves, so they can't be skipped.

https://codereview.chromium.org/1811333002/diff/100001/src/compiler/schedule.cc
File src/compiler/schedule.cc (right):

https://codereview.chromium.org/1811333002/diff/100001/src/compiler/schedule....
src/compiler/schedule.cc:324: replace_if(pred->successors().begin(),
pred->successors().end(),
A regular for loop would be more readable here.

https://codereview.chromium.org/1811333002/diff/100001/src/compiler/schedule....
src/compiler/schedule.cc:342: // and other improperly maked blocks until a fixed
point is reached.
s/maked/marked

https://codereview.chromium.org/1811333002/diff/100001/src/compiler/schedule....
src/compiler/schedule.cc:343: // TODO(danno): optimize the propagaion
s/propagaion/propagation

[danno, 2016-03-22 07:53:44]

Feedback addressed. Please take another look.

https://codereview.chromium.org/1811333002/diff/100001/src/compiler/instructi...
File src/compiler/instruction-selector.cc (right):

https://codereview.chromium.org/1811333002/diff/100001/src/compiler/instructi...
src/compiler/instruction-selector.cc:810: // The scheduler may have inserted a
split-edge block between the switch
On 2016/03/21 08:06:03, titzer wrote:
> Not sure this is sound. The split-edge blocks are the placement points for phi
> resolution moves, so they can't be skipped.

As discussed, moved the logic to ensure the extra edge to the Switch-building
logic in the RawMachineAssembler.

https://codereview.chromium.org/1811333002/diff/100001/src/compiler/schedule.cc
File src/compiler/schedule.cc (right):

https://codereview.chromium.org/1811333002/diff/100001/src/compiler/schedule....
src/compiler/schedule.cc:324: replace_if(pred->successors().begin(),
pred->successors().end(),
On 2016/03/21 08:06:03, titzer wrote:
> A regular for loop would be more readable here.

Done.

https://codereview.chromium.org/1811333002/diff/100001/src/compiler/schedule....
src/compiler/schedule.cc:342: // and other improperly maked blocks until a fixed
point is reached.
On 2016/03/21 08:06:03, titzer wrote:
> s/maked/marked

Done.

https://codereview.chromium.org/1811333002/diff/100001/src/compiler/schedule....
src/compiler/schedule.cc:343: // TODO(danno): optimize the propagaion
On 2016/03/21 08:06:03, titzer wrote:
> s/propagaion/propagation

Done.

[titzer, 2016-03-23 07:27:14]

On 2016/03/22 07:53:44, danno wrote:
> Feedback addressed. Please take another look.
> 
>
https://codereview.chromium.org/1811333002/diff/100001/src/compiler/instructi...
> File src/compiler/instruction-selector.cc (right):
> 
>
https://codereview.chromium.org/1811333002/diff/100001/src/compiler/instructi...
> src/compiler/instruction-selector.cc:810: // The scheduler may have inserted a
> split-edge block between the switch
> On 2016/03/21 08:06:03, titzer wrote:
> > Not sure this is sound. The split-edge blocks are the placement points for
phi
> > resolution moves, so they can't be skipped.
> 
> As discussed, moved the logic to ensure the extra edge to the Switch-building
> logic in the RawMachineAssembler.
> 
>
https://codereview.chromium.org/1811333002/diff/100001/src/compiler/schedule.cc
> File src/compiler/schedule.cc (right):
> 
>
https://codereview.chromium.org/1811333002/diff/100001/src/compiler/schedule....
> src/compiler/schedule.cc:324: replace_if(pred->successors().begin(),
> pred->successors().end(),
> On 2016/03/21 08:06:03, titzer wrote:
> > A regular for loop would be more readable here.
> 
> Done.
> 
>
https://codereview.chromium.org/1811333002/diff/100001/src/compiler/schedule....
> src/compiler/schedule.cc:342: // and other improperly maked blocks until a
fixed
> point is reached.
> On 2016/03/21 08:06:03, titzer wrote:
> > s/maked/marked
> 
> Done.
> 
>
https://codereview.chromium.org/1811333002/diff/100001/src/compiler/schedule....
> src/compiler/schedule.cc:343: // TODO(danno): optimize the propagaion
> On 2016/03/21 08:06:03, titzer wrote:
> > s/propagaion/propagation
> 
> Done.

lgtm

[danno, 2016-03-23 07:32:20]

The CQ bit was checked by danno@chromium.org

[commit-bot: I haz the power, 2016-03-23 07:32:26]

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

[commit-bot: I haz the power, 2016-03-23 07:34:44]

Description was changed from

==========
Ensure Schedules generated by the RawMachineAssembler are in edge-split form

This CL adds an extra pass before calculating the special RPO
order in the custom RawMachineAssembler pipeline that
walks through the schedule and inserts extra blocks to 
guarantee that the control flow graph is in split edge form. It
also propagates deferred block marks forward to these new
blocks if appropriate.
==========

to

==========
Ensure Schedules generated by the RawMachineAssembler are in edge-split form

This CL adds an extra pass before calculating the special RPO
order in the custom RawMachineAssembler pipeline that
walks through the schedule and inserts extra blocks to 
guarantee that the control flow graph is in split edge form. It
also propagates deferred block marks forward to these new
blocks if appropriate.
==========

[commit-bot: I haz the power, 2016-03-23 07:34:44]

Committed patchset #7 (id:120001)

[commit-bot: I haz the power, 2016-03-23 07:37:30]

Description was changed from

==========
Ensure Schedules generated by the RawMachineAssembler are in edge-split form

This CL adds an extra pass before calculating the special RPO
order in the custom RawMachineAssembler pipeline that
walks through the schedule and inserts extra blocks to 
guarantee that the control flow graph is in split edge form. It
also propagates deferred block marks forward to these new
blocks if appropriate.
==========

to

==========
Ensure Schedules generated by the RawMachineAssembler are in edge-split form

This CL adds an extra pass before calculating the special RPO
order in the custom RawMachineAssembler pipeline that
walks through the schedule and inserts extra blocks to
guarantee that the control flow graph is in split edge form. It
also propagates deferred block marks forward to these new
blocks if appropriate.

Committed: https://crrev.com/a827400025be77a75880b1255a269df52dca103d
Cr-Commit-Position: refs/heads/master@{#35014}
==========

[commit-bot: I haz the power, 2016-03-23 07:37:31]

Patchset 7 (id:??) landed as
https://crrev.com/a827400025be77a75880b1255a269df52dca103d
Cr-Commit-Position: refs/heads/master@{#35014}