[PATCH] D57316: [LoopSimplifyCFG] Fix DT update strategy

[Max Kazantsev via Phabricator via llvm-commits]

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