[Dominators] Facilitate updates to MachinePostDominatorTree

[critson]

Summary

Add getBase accessor so that underlying tree can be
manipulated in a similar manner to MachineDominatorTree.

[kuhar]

Could you add some basic unit tests and a separate set of unit tests where critical edges get split?

[critson]

Add basic unit tests.

[nhaehnle]

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...)

[critson]

In 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.

[critson]

Simplify the change set to bare minimum required.

[critson]

Upload correct diff.

[kuhar]

LGTM.

While this is fine, I'm I don't understand how splitting critical edges will interact with domtree updates when you call getBase in MachineDominators.h. But we can discuss that in another patch.

llvm/include/llvm/CodeGen/MachinePostDominators.h

class MachinePostDominatorTree : public MachineFunctionPass {
   using PostDomTreeT = PostDomTreeBase<MachineBasicBlock>;
   std::unique_ptr<PostDomTreeT> PDT;
 
 public:
   static char ID;
 
   MachinePostDominatorTree();
 
+  PostDomTreeT &getBase() {
+    if (!PDT)
+      PDT.reset(new PostDomTreeT());
+    return *PDT;
+  }
+
   FunctionPass *createMachinePostDominatorTreePass();
 
   const SmallVectorImpl<MachineBasicBlock *> &getRoots() const {
     return PDT->getRoots();
   }
 
   MachineDomTreeNode *getRootNode() const { return PDT->getRootNode(); }