[PATCH] D68235: [Dominators][CodeGen] Add MachinePostDominatorTree verification

[Jakub Kuderski via Phabricator via llvm-commits]

kuhar marked an inline comment as done.
kuhar added inline comments.


================
Comment at: llvm/trunk/lib/CodeGen/MachineSink.cpp:124
       AU.addRequired<MachineBranchProbabilityInfo>();
-      AU.addPreserved<MachineDominatorTree>();
-      AU.addPreserved<MachinePostDominatorTree>();
       AU.addPreserved<MachineLoopInfo>();
       if (UseBlockFreqInfo)
----------------
uabelho wrote:
> Hi,
> 
> In my out-of-tree target I'm seeing strange problems with MachineLoopInfo with this patch and I'm wondering if perhaps we should also remove MachineLoopInfo from the preserved passes above?
> 
> Since MachineLoopInfo uses MachineDominatorTree, won't MLI also possibly be invalid if MDT is invalid?
> 
Hi Mikael,

Over the last couple of days I found a few passes that modify CFG and do not update dominators while claiming to preserve them, which all should be fixed now.

I'm not familiar with MachineLI, but skimming the code it seems that any pass that introduces new blocks and doesn't add it to MLI invalidates it. This seems to be the case for MachineSink, PHIElimination, and maybe some other passes I missed. I think that in those cases the passes have to be fixes to not say they preserve MLI.

Independently, perhaps it would be worth/possible to implement `verifyAnalysis` for MLI and see where it currently breaks?



Repository:
  rL LLVM

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

https://reviews.llvm.org/D68235