[PATCH] D49925: [SimpleLoopUnswitch] Fix DT updates for trivial branch unswitching.

[Chandler Carruth via Phabricator via llvm-commits]

chandlerc added a comment.

In https://reviews.llvm.org/D49925#1178887, @kuhar wrote:

> @chandlerc FYI, all existing uses of the 'raw' api (DT.insert/deleteEdge, DT.applyUpdates) are going to be replaced with the new unified API after branching to 8.0. @NutshellySima  is working on this.
>  The plan is also to hide `insert/deleteEdge` completely, so that we won't have bugs like this in the future.


Sure, but a *bunch* of code is going to use these APIs before that point, so I think we should still invest in their documentation.

While the comments on `insertEdge` and `deleteEdge` try to make the correctness issues clear, if you don't want folks to use them, they could be much more clear about that.

The comment on `applyUpdates` that says it should be faster than calling `insertEdge` and `deleteEdge` directly is the one that confused me -- it makes it sound like this is just a performance tradeoff. Could improve it just by reminding the reader that there is a bigger semantic difference. Something like "In addition to supporting a batch of updates (which direct calls don't), this is more efficient than directly applying the updates ...." or something.


Repository:
  rL LLVM

https://reviews.llvm.org/D49925