[PATCH] D32614: [GVNHoist] Fix: PR32821, add check for anticipability in case of infinite loops

[Chandler Carruth via Phabricator via llvm-commits]

chandlerc requested changes to this revision.
chandlerc added a comment.
This revision now requires changes to proceed.

I don't see what would have addressed my concerns.

There still is only a single test case for an infinite loop. There are *several different* CFGs that fail to terminate. You should be testing them.

Examples off the top of my head:

- A non-loop cycle in the CFG. Note that you need to set up the entire hoisting to be *inside* of the outer loop and then make it invalid due to an infinite non-loop cycle that exists within the loop.
- A *potential* cycle due to indirect-br
- An exceptional cycle due to cyclic exception edges from invokes
- Either indirect-br or exceptions combined with a non-loop cycle.

I understand that you may naturally have an algorithm that handles all of these without special casing. That is good! But you should *test* them to ensure that when your code sees unexpected or rarely formed control flows it continues to behave well.

Put differently, it would be good to add fairly extensive testing that actively tries to construct corner cases to break the safety checks of this pass as it has had a long history of subtle and difficult to find correctness bugs.

Have you or anyone that cares about GVN hoist considered building a tool to synthetically generate random CFGs with some properties to see how the pass behaves? That might help more pro-actively find issues.


https://reviews.llvm.org/D32614