[PATCH] D43293: [SimplifyCFG] Don't remove preheaders when we need canonical loops.

[Serguei Katkov via Phabricator via llvm-commits]

skatkov added a comment.

In https://reviews.llvm.org/D43293#1011553, @dmgreen wrote:

> > When you remove backedge you introduce new backedge. So you need to add new backedge to the list of the backedges.
>
> I looked into this a little, and had convinced myself that it would be OK (if we add a new backedge, we probably don't want to remove it again in the same pass). Looking at it again though, that might not be correct. I've not done much work in simplify cfg before. Do you think this would involve updating the backedge list in many places?
>
> Perhaps we can turn this around, calculate the Preheaders as `Preds(Headers) - Backedges` and use that instead.


I do not follow how computing Preheaders as `Preds(Headers) - Backedges` will help you.
In general, potentially you can have CFG like empty backedge coming to pre-header only and predecessor of backedge is also empty block having only one predecessor. In this case current implementation will eliminate only one backedge and preserve the second one.

In my understanding each time you are removing backedge you need add all its predecessors to backedge if this backedge is not a loop header. The last case seems to me impossible (otherwise you eliminate the loop and can be asserted). 
To minimize changes you can introduce the utility which will do this check (and even you can put LoopHeader erase to the same utility function) and everywhere you invoke backedge erase you can invoke this utility..


https://reviews.llvm.org/D43293