[PATCH] D48919: [Dominators] Convert existing passes to use DomTreeUpdater - 1

[Chijun Sima via Phabricator via llvm-commits]

NutshellySima added a comment.

In https://reviews.llvm.org/D48919#1153192, @brzycki wrote:

> In https://reviews.llvm.org/D48919#1152530, @kuhar wrote:
>
> > One thing that is not clear to me is whether it's work to replace plain updates (DT->insert/deleteEdge, DT->applyUpdates) with DTU when it's clear that both are going to be functionally the same. The overhead of constructing DTU is probably negligible, but it's more verbose and I'm not sure what benefit it brings.
> >
> > It definitely makes sense to use DTU when you are updating both DT and PDT, and when you can use the lazy strategy, or use some other functions that take DTU. Do you think it's worth to replace it everywhere for some other reason?
>
>
> Hi @kuhar, I was hoping all instances of DT were replaced with DTU for the following reasons:
>
> - call consistency. IT reduces problems when users do a `git grep` to see how others call DT and being unsure which interface to use, and why.
> - every call site of DTU gets DT and PDT for free simply by adding the tree to the object. It makes both types of analysis easier to add in and potentially more used.
> - an abstraction layer to the DT and PDT trees giving more flexibility to change the mechanics of both should new algorithms be developed or performance enhancements discovered.
> - potential performance improvements. We can test eager vs lazy in different subsystems to tune the creation and use of DT/PDT.
> - easier to add DT/PDT preservation to other passes. One of the most difficult parts of preserving DT in JumpThreading was all the external calls to Local, BasicBlock, or BasicBlockUtils. Had those routunes passed in a DTU instead of a DT the changes would be smaller and simpler. There are one or two calls in JumpThreading where I manually fixed things up before and after the external call simply because it was less-invasive and required far less testing than changing the external call. I also ran into problem with header inclusion problems when working with such core parts of the middle-end.
>
>   It's my hope, unless I'm missing something here, to eventually see very few parameterized method/function calls containing a  DT/PDT pointer in llvm.


Hi @brzycki, thanks for pointing them out. It is good to see passes getting easier DT/PDT preservation by replacing  (DT->insert/deleteEdge, DT->applyUpdates)  to corresponding ones in DTU and adding PDT to the object. I'll add them back in https://reviews.llvm.org/D48967.


Repository:
  rL LLVM

https://reviews.llvm.org/D48919