[PATCH] D11683: Create a wrapper pass for MachineDominatorTree to remove pass dependence when it is only used conditionally.

[Duncan P. N. Exon Smith]

> On 2015-Jul-31, at 10:44, Cong Hou <congh at google.com> wrote:
> 
> congh created this revision.
> congh added reviewers: dexonsmith, dblaikie.
> congh added subscribers: llvm-commits, davidxl.
> Herald added subscribers: qcolombet, MatzeB.
> 
> This patch adds a wrapper pass MachineDominatorTreeWrapperPass that contains MachineDominatorTree analysis, which could be done flexibly without declaring pass dependence. This is good to the compilation time: in some cases we only need to do this analysis conditionally (like in MachineBlockPlacement in this patch). This is quite similar to the wrapper pass for DominatorTree.
> 
> Most changes are mechanical due to the name and usage changes.
> 
> http://reviews.llvm.org/D11683
> 
> Files:
>  include/llvm/CodeGen/MachineDominators.h
>  include/llvm/InitializePasses.h
>  lib/CodeGen/CodeGen.cpp
>  lib/CodeGen/EarlyIfConversion.cpp
>  lib/CodeGen/InlineSpiller.cpp
>  lib/CodeGen/LiveDebugVariables.cpp
>  lib/CodeGen/LiveIntervalAnalysis.cpp
>  lib/CodeGen/MachineBasicBlock.cpp
>  lib/CodeGen/MachineBlockPlacement.cpp
>  lib/CodeGen/MachineCSE.cpp
>  lib/CodeGen/MachineCombiner.cpp
>  lib/CodeGen/MachineDominanceFrontier.cpp
>  lib/CodeGen/MachineDominators.cpp
>  lib/CodeGen/MachineLICM.cpp
>  lib/CodeGen/MachineLoopInfo.cpp
>  lib/CodeGen/MachineRegionInfo.cpp
>  lib/CodeGen/MachineScheduler.cpp
>  lib/CodeGen/MachineSink.cpp
>  lib/CodeGen/PHIElimination.cpp
>  lib/CodeGen/PeepholeOptimizer.cpp
>  lib/CodeGen/PostRASchedulerList.cpp
>  lib/CodeGen/PrologEpilogInserter.cpp
>  lib/CodeGen/RegAllocBasic.cpp
>  lib/CodeGen/RegAllocGreedy.cpp
>  lib/CodeGen/RegAllocPBQP.cpp
>  lib/CodeGen/ShrinkWrap.cpp
>  lib/CodeGen/StackColoring.cpp
>  lib/CodeGen/UnreachableBlockElim.cpp
>  lib/Target/AArch64/AArch64CleanupLocalDynamicTLSPass.cpp
>  lib/Target/AArch64/AArch64CollectLOH.cpp
>  lib/Target/AArch64/AArch64ConditionOptimizer.cpp
>  lib/Target/AArch64/AArch64ConditionalCompares.cpp
>  lib/Target/AMDGPU/AMDILCFGStructurizer.cpp
>  lib/Target/AMDGPU/R600OptimizeVectorRegisters.cpp
>  lib/Target/AMDGPU/R600Packetizer.cpp
>  lib/Target/AMDGPU/SIFoldOperands.cpp
>  lib/Target/AMDGPU/SILowerI1Copies.cpp
>  lib/Target/Hexagon/HexagonFrameLowering.cpp
>  lib/Target/Hexagon/HexagonGenInsert.cpp
>  lib/Target/Hexagon/HexagonGenPredicate.cpp
>  lib/Target/Hexagon/HexagonHardwareLoops.cpp
>  lib/Target/Hexagon/HexagonVLIWPacketizer.cpp
>  lib/Target/Mips/MipsOptimizePICCall.cpp
>  lib/Target/PowerPC/PPCCTRLoops.cpp
>  lib/Target/SystemZ/SystemZLDCleanup.cpp
>  lib/Target/X86/X86InstrInfo.cpp
> 
> <D11683.31132.patch>

I have some comments on the MachineBlockPlacement changes (I'm not
comfortable signing off even after you address my comments, though,
so someone else will have to chime in).

> Index: lib/CodeGen/MachineBlockPlacement.cpp
> ===================================================================
> --- lib/CodeGen/MachineBlockPlacement.cpp
> +++ lib/CodeGen/MachineBlockPlacement.cpp
> @@ -202,7 +202,7 @@
>    const TargetLoweringBase *TLI;
>  
>    /// \brief A handle to the post dominator tree.
> -  MachineDominatorTree *MDT;
> +  std::unique_ptr<MachineDominatorTree> MDT;

I don't see you releasing this memory anywhere.  Should you?

>  
>    /// \brief A set of blocks that are unavoidably execute, i.e. they dominate
>    /// all terminators of the MachineFunction.
> @@ -262,7 +262,6 @@
>    void getAnalysisUsage(AnalysisUsage &AU) const override {
>      AU.addRequired<MachineBranchProbabilityInfo>();
>      AU.addRequired<MachineBlockFrequencyInfo>();
> -    AU.addRequired<MachineDominatorTree>();

If the MDT is already available, shouldn't we use it?  Should this be
changed to `addOptional<>`?

>      AU.addRequired<MachineLoopInfo>();
>      MachineFunctionPass::getAnalysisUsage(AU);
>    }
> @@ -275,7 +274,6 @@
>                        "Branch Probability Basic Block Placement", false, false)
>  INITIALIZE_PASS_DEPENDENCY(MachineBranchProbabilityInfo)
>  INITIALIZE_PASS_DEPENDENCY(MachineBlockFrequencyInfo)
> -INITIALIZE_PASS_DEPENDENCY(MachineDominatorTree)
>  INITIALIZE_PASS_DEPENDENCY(MachineLoopInfo)
>  INITIALIZE_PASS_END(MachineBlockPlacement, "block-placement",
>                      "Branch Probability Basic Block Placement", false, false)
> @@ -1138,7 +1136,8 @@
>    MLI = &getAnalysis<MachineLoopInfo>();
>    TII = F.getSubtarget().getInstrInfo();
>    TLI = F.getSubtarget().getTargetLowering();
> -  MDT = &getAnalysis<MachineDominatorTree>();
> +  if (OutlineOptionalBranches)
> +    MDT.reset(new MachineDominatorTree(F));
>    assert(BlockToChain.empty());
>  
>    buildCFGChains(F);
>