Utility function that we use for blocks deletion always unconditionally removes
one-input Phis. In LoopSimplifyCFG, it can lead to breach of LCSSA form.
This patch alters this function to keep them if needed.
Details
Diff Detail
- Repository
- rL LLVM
Event Timeline
I think ideally we would just avoid deleting LCSSA PHI nodes (PHI nodes with a single incoming value in loop exit blocks), but maybe it is too much effort with the info available.
include/llvm/Transforms/Utils/BasicBlockUtils.h | ||
---|---|---|
44 ↗ | (On Diff #183518) | nit: How about KeepUselessPHIs, or KeepSingleValuePHIs, which is a bit more descriptive. |
test/Transforms/LoopSimplifyCFG/lcssa.ll | ||
2 ↗ | (On Diff #183518) | I think the relevant -verify-loop-lcssa & co are available without asserts as well, so we should be able to drop REQUIRE: asserts. |
include/llvm/Transforms/Utils/BasicBlockUtils.h | ||
---|---|---|
44 ↗ | (On Diff #183518) | I used exactly the same name as existing function removePredecessor has. We can make a follow-up refactoring to change them all, but so far I'd prefer to keep things uniform. |
test/Transforms/LoopSimplifyCFG/lcssa.ll | ||
2 ↗ | (On Diff #183518) | I didn't check that. We can do it as follow-up if it works. |
LGTM, but please, schedule those followups discussed here.
include/llvm/Transforms/Utils/BasicBlockUtils.h | ||
---|---|---|
44 ↗ | (On Diff #183518) | I also find this name very confusing... |
include/llvm/Transforms/Utils/BasicBlockUtils.h | ||
---|---|---|
44 ↗ | (On Diff #183518) | I will rename all its occurrences in a follow-up. |
I'll split out changes in basic block utils as separate NFC, then submit change in LoopSimplifyCFG separately.
include/llvm/Transforms/Utils/BasicBlockUtils.h | ||
---|---|---|
44 ↗ | (On Diff #183518) |