[PATCH] D57316: [LoopSimplifyCFG] Fix DT update strategy
Max Kazantsev via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Mon Jan 28 03:06:52 PST 2019
mkazantsev added a comment.
I think the underlying problem is a bug of DTU, but am not 100% sure. In the given test, eager strategy does the following:
1. Removes dead edge `bb4->bb18`;
2. Tries to remove edge `bb18->bb6`, but the actual deletion does not happen because `bb18` does not have a DT node;
3. As result, after these two operations the IDOM of `bb6` is `bb3`, while it should be `bb29` after we have removed edge `bb18->bb6`.
To me it seems that at some point we screwed up making the probper updates for `bb6`. I think we should've done it when removed `bb4->bb18`. However the comments in DTU about that are fuzzy. They say that:
/// Notify all available trees on an edge deletion. If both DT and PDT are
/// nullptrs, this function discards the update. Under either Strategy,
/// self-dominance update will be removed. The Eager Strategy applies
/// the update immediately while the Lazy Strategy queues the update.
/// It is recommended to only use this method when you have exactly one
/// deletion (and no insertions). It is recommended to use applyUpdates() in
/// all other cases. This function has to be called *after* making the update
/// on the actual CFG. An internal functions checks if the edge doesn't exist
/// in the CFG in DEBUG mode.
void deleteEdge(BasicBlock *From, BasicBlock *To);
I do not know what this recommendation is about. Does anybody know whether DTU should be able to work correctly with eager strategy?
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D57316/new/
https://reviews.llvm.org/D57316
More information about the llvm-commits
mailing list