[PATCH] D77967: [Dominators] Facilitate updates to MachineDominator trees

Carl Ritson via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Apr 14 03:41:15 PDT 2020


critson added a comment.

In D77967#1980065 <https://reviews.llvm.org/D77967#1980065>, @nhaehnle wrote:

> Couldn't whichever user of MachineDominatorTree needs this just access the underlying dominator tree structures via the `getBase()` accessor method and call the relevant functions on there?
>
> The reason I'm concerned about this is that the current MachineDominatorTree falls out of the more common pattern for analyses that has been established for the new pass manager. If you'll look at the dominator tree on IR, for example, you'll see that there's a `DominatorTree` class that is not a pass in any way, and then DominatorTreeWrapperPass (old pass manager) and DominatorTreeAnalysis (new pass manager) wrapper passes that will return the `DominatorTree` object.
>
> I think it would be better if the MachineDominatorTree usage would be better aligned with that pattern, rather than moving further away from it. (Currently, MachineDominatorTree has the custom critical-edge-split handling which should arguably be refactored into a use of the general dominator tree-updater mechanism as well...)


Having spent a while looking at it recently, I agree the interface for MachineDominatorTree is definitely uncomfortably far from the DominatorTree interface and fixing that would be a good thing.

Looking into it I can refactor the changes I had in mind to use `getBase()`, although it is missing for MachinePostDominatorTree.
So I can reduce this change to adding a `getBase()` accessor for MachinePostDominatorTree.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D77967/new/

https://reviews.llvm.org/D77967





More information about the llvm-commits mailing list