Remove jumps over empty blocks.

Remove jumps over empty blocks.

After contracting a node reorder it as unreachable even if it
hasn't yet been placed.

BUG=
R=jvoung@chromium.org, jvoung, stichnot

Committed: https://gerrit.chromium.org/gerrit/gitweb?p=native_client/pnacl-subzero.git;a=commit;h=713278ab2a513b99c1b972ed03876a4c8462552b

[jvoung (off chromium), 2015-07-21 23:27:54]

You mentioned trying to remove the Contracted nodes from the list and compacting
the list, but that ran into problems later with the assembler(?) asserting that
some cfgnode entry-point labels were left unbound? At what stage did you try to
add that pass of removing?

Contracting also clears the InEdges, so it essentially makes the node a
candidate to be "unreachable". The Cfg::reorderNodes tracks Unreachable blocks
and stashes them at the end, but only does that for nodes that "needsPlacement".
I wonder if the check for unreachable could happen first, then the check for
needsPlacement happens later. Could that work?

[ascull, 2015-07-22 23:26:30]

I changed it to use reordering instead of a contracted flag.
The branch optimization was also deleting *all* the
unconditional branches to the default case which is also fixed.

[jvoung (off chromium), 2015-07-23 00:01:23]

https://codereview.chromium.org/1246173003/diff/60001/src/IceCfgNode.cpp
File src/IceCfgNode.cpp (right):

https://codereview.chromium.org/1246173003/diff/60001/src/IceCfgNode.cpp#newc...
src/IceCfgNode.cpp:732: assert(OutEdges.size() == 1);
Might be cleaner to "cache" OutEdges.front() as "Succ", since that's used in
quite a few places now, and you have to look at this assert to realize why only
OutEdges.front() is referenced.

https://codereview.chromium.org/1246173003/diff/60001/src/IceCfgNode.cpp#newc...
src/IceCfgNode.cpp:768: // block. Take case with switch lowering as there are
multiple unconditional
What do you mean by "Take case with switch lowering" ?

[ascull, 2015-07-23 00:11:20]

https://codereview.chromium.org/1246173003/diff/60001/src/IceCfgNode.cpp
File src/IceCfgNode.cpp (right):

https://codereview.chromium.org/1246173003/diff/60001/src/IceCfgNode.cpp#newc...
src/IceCfgNode.cpp:732: assert(OutEdges.size() == 1);
On 2015/07/23 00:01:23, jvoung wrote:
> Might be cleaner to "cache" OutEdges.front() as "Succ", since that's used in
> quite a few places now, and you have to look at this assert to realize why
only
> OutEdges.front() is referenced.

Done.

https://codereview.chromium.org/1246173003/diff/60001/src/IceCfgNode.cpp#newc...
src/IceCfgNode.cpp:768: // block. Take case with switch lowering as there are
multiple unconditional
On 2015/07/23 00:01:23, jvoung wrote:
> What do you mean by "Take case with switch lowering" ?

I mean 'take care'. Too much switching!

[jvoung (off chromium), 2015-07-23 00:14:42]

LGTM thanks =)

[ascull, 2015-07-23 00:17:05]

Committed patchset #5 (id:80001) manually as
713278ab2a513b99c1b972ed03876a4c8462552b (presubmit successful).