[PATCH] D61055: [LoopSimplifyCFG] Suppress expensive DomTree verification in LoopSimplifyCFG

[Dave Green via Phabricator via llvm-commits]

dmgreen added a comment.

> That being said, I don't think that people should asserting correctness of domtree within transforms, unless there's a strong need. This is generally not cheap, even at the Fast level, and because of that I think that if someone really wants to have a fast domtree verification in an assertion, they should make it explicit. Historically, Fast had been the default, and the running time penalty started to pile up, as many places assumed it's very cheap to perform. It's much easier to realize these assertions are fishy if they are not usually-cheap-but-not-quite by default.

My understanding of the problem is that many people (who quite possibly do not know the ins and outs of dominators) update passes to preserve the domtree. They need to verify that what they have done is correct, so they add an assert(DT.verify()); This is a good thing and it catches problems that are often easy to make with DT updating, like missing edge inserts/removals. They often don't realise that the default is very heavy-weight though, and expect it to work more like LI->verify or isRecursivelyLCSSAForm.

So the argument for fast being the default would be that the users who don't necessarily know better get a good default option. And the people who do know better can use Basic/Full.

But I was not aware that Fast used to be the default, and was still quite slow. I was under the impression that the problems in the past were to do with it using Full under expensive checks, and that Fast was fairly quick (quick enough for an asserts build).


Repository:
  rL LLVM

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

https://reviews.llvm.org/D61055