[PATCH] D47441: SafepointIRVerifier should ignore dead blocks and dead edges

[Anna Thomas via Phabricator via llvm-commits]

anna added inline comments.


================
Comment at: lib/IR/SafepointIRVerifier.cpp:242
+/// without splitting critical edges. So the CFG is kept intact.
+class DeadBlocks : public SetVector<const BasicBlock *> {
+  const DominatorTree &DT;
----------------
apilipenko wrote:
> anna wrote:
> > This seems like a generally useful analysis to have outside of the safepoint IR Verifier. We should think of moving the code into Analysis. 
> > 
> > You mentioned it's taken from the GVN but modified to avoid splitting the critical edges. Could you please state that as a comment here: modified version from `GVN::addDeadBlock`. 
> > 
> @anna, I think this is not of much use outside of SafepointIRVerifier. Most passes don't need to worry about this patterns as it's reasonable to expect simplifycfg to have cleaned things up.
> 
> Verifiers is a different story as you don't want you verification logic depend on the optimizations performed on the code. You also don't want to modify the code to clean up the mess.
> 
> I would suggest keeping this logic in SafepointIRVerifier. If we want to extract it as a separate analysis I'd suggest making a clear comment that it has very specific purpose.
There's one specific use for this pass outside of the SafepointIRVerifier and that's for loopSimplifyCFG - we cannot rely on SimplifyCFG to cleanup dead/unreachable blocks that are dead *because* of LoopSimplifyCFG. So, I suspect this will be useful for loop optimizations that are run as a group in a pipeline without the need for a function pass like `SimplifyCFG`. 

Perhaps we can add a clear comment stating where it's usage is expected and that right now it's used in the SafepointIRVerifier.




https://reviews.llvm.org/D47441