[PATCH] D14996: [WinEH] Avoid infinite loop in BranchFolding for multiple single block funclets

[Matthias Braun via llvm-commits]

MatzeB added a comment.

In http://reviews.llvm.org/D14996#475871, @andrew.w.kaylor wrote:

> The purpose of this change was to avoid a case where the old algorithm was getting into an infinite loop moving different blocks to the end of the function.  As I recall, this situation arose because the old code was sometimes mistakenly deciding that control flow could fall through from a normal block to an EH block that was reachable from that block via an exception.  If multiple EH blocks were reachable, the old code wanted to place each of them immediately after the block from which they were reached.
>
> But basically I think you're right that this is inherently an optimization in as much as the change here was meant to preserve the code that was trying to move a non-EH fall through block into an optimized position while avoiding the infinite loop problem.
>
> As to the test case not failing without the code change for ToT, it looks like when the EH IR was redefined and the test case was updated, it no longer met the conditions that triggered the infinite loop.  I'm not sure it isn't possible to create the conditions for the infinite loop though.
>
> I think the safest change would be to eliminate the code I introduced to skip over EH blocks but add a condition so that it didn't move the Fallthrough block if it is an EH block.
>
>   if (FallThrough != MF.end() &&
>       !TII->AnalyzeBranch(PrevBB, PrevTBB, PrevFBB, PrevCond, true) &&
>
> - PrevBB.isSuccessor(&*FallThrough)) { +         PrevBB.isSuccessor(&*FallThrough) && +         !FallThrough->isEHPad()) {
>
>   I'll play around with the test case and see if I can get it back to where it fails without this change.


Any news on this?


Repository:
  rL LLVM

http://reviews.llvm.org/D14996