[llvm-commits] [PATCH] Restructure a part of SimplifyCFG

[Matthijs Kooijman]

Hi All,

attached is a patch that restructures a part of the SimplifyCFG pass.

The SimplifyCFG pass looks at basic blocks that contain only phi nodes,
followed by an unconditional branch. In a lot of cases, such a block (BB) can
be merged into their successor (Succ).

This merging is performed by TryToSimplifyUncondBranchFromEmptyBlock. It does
this by taking all phi nodes in the succesor block Succ and expanding them to
include the predecessors of BB. Furthermore, any phi nodes in BB are moved to
Succ and expanded to include the predecessors of Succ as well.

Before attempting this merge, CanPropagatePredecessorsForPHIs checks to see if
all phi nodes can be properly merged. All functional changes have been made to
this function, I only updated comments in
TryToSimplifyUncondBranchFromEmptyBlock.

In the original code, CanPropagatePredecessorsForPHIs looks quite convoluted
and more like stack of checks added to handle different kinds of situations
than a comprehensive check. In particular the first check in the function did
some value checking for the case that BB and Succ have a common predecessor,
while the last check in the function simply rejected all cases where BB and
Succ have a common predecessor. The first check was still useful in the case
that BB did not contain any phi nodes at all, though, so it was not completely
useless.

However, in my patch I've restructured the CanPropagatePredecessorsForPHIs
function to look a lot more similar to the code that actually performs the
merge. Both functions now look at the same phi nodes in about the same order.
Any conflicts (phi nodes with different values for the same source) that could
arise from merging or moving phi nodes are detected. If no conflicts are
found, the merge can happen.

Apart from only restructuring the checks, two main changes in functionality
happened.

Firstly, the old code rejected blocks with common predecessors in most cases.
The new code performs some extra checks so common predecessors can be handled
in a lot of cases. Wherever common predecessors still pose problems, the
blocks are left untouched.

Secondly, the old code rejected the merge when values (phi nodes) from BB were
used in any other place than Succ. However, after giving this some extensive
thought, I cannot think of any situation that would require this check. Even
more, I think this can be proven.

Consider that BB is a block containing of a single phi node "%a" and a branch
to Succ. Now, since the definition of %a will dominate all of its uses, BB
will dominate all blocks that use %a. Furthermore, since the branch from BB to
Succ is unconditional, Succ will also dominate all uses of %a.

Now, assume that one predecessor of Succ is not dominated by BB (and thus not
dominated by Succ). Since at least one use of %a (but in reality all of them)
is reachable from Succ, you could end up at a use of %a without passing
through it's definition in BB (by coming from X through Succ). This is a
contradiction, meaning that our original assumption is wrong. Thus, all
predecessors of Succ must also be dominated by BB (and thus also by Succ).

This means that moving the phi node %a from BB to Succ does not pose any
problems when the two blocks are merged, and any use checks are not needed.

I'm also working on a few testcases to properly illustrate the changes by this
patch, I will send them in a seperate email later today.

Gr.

Matthijs
-------------- next part --------------
A non-text attachment was scrubbed...
Name: simplifycfg.diff
Type: text/x-diff
Size: 9850 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20080516/8486f2b9/attachment.diff>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 189 bytes
Desc: Digital signature
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20080516/8486f2b9/attachment.sig>