[PATCH] D94827: [SimplifyCFG] Require and preserve dominator tree

2021-01-18 Thread Max Kazantsev via Phabricator via cfe-commits
mkazantsev added inline comments. Comment at: llvm/lib/Transforms/Scalar/SimplifyCFGPass.cpp:263 const SimplifyCFGOptions &Options) { - assert((!RequireAndPreserveDomTree || - (DT && DT->verify(DominatorTree::VerificationLevel::Full))) &

[PATCH] D94827: [SimplifyCFG] Require and preserve dominator tree

2021-01-18 Thread Nikita Popov via Phabricator via cfe-commits
nikic added a comment. In D94827#2505431 , @lebedev.ri wrote: > Wow. So much polarization here. > > In D94827#2502435 , @kuhar wrote: > >> Wow, this is fantastic. When I first started working on the domtree updater

[PATCH] D94827: [SimplifyCFG] Require and preserve dominator tree

2021-01-18 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added a comment. Wow. So much polarization here. In D94827#2502435 , @kuhar wrote: > Wow, this is fantastic. When I first started working on the domtree updater > back in 2017, SimplifyGFG seemed like one of the most difficult passes to > han

[PATCH] D94827: [SimplifyCFG] Require and preserve dominator tree

2021-01-18 Thread Dávid Bolvanský via Phabricator via cfe-commits
xbolva00 added a comment. >> Code complexity Was there any RFC or general agreement? After many patches for this goal the pass is now more complex and harder to read.. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D94827/new/ https://reviews.llvm

[PATCH] D94827: [SimplifyCFG] Require and preserve dominator tree

2021-01-18 Thread Nikita Popov via Phabricator via cfe-commits
nikic added a comment. I'm somewhat skeptical about this change. The motivation seems a bit weak given the costs involved. The costs are: - Compile-time cost. Your best case estimate puts this at a 0.5% compile-time regression. This is for the case where SimplifyCFG simplify preserves DT, with

[PATCH] D94827: [SimplifyCFG] Require and preserve dominator tree

2021-01-18 Thread Max Kazantsev via Phabricator via cfe-commits
mkazantsev added a comment. This is awesome. And scary. This is scarily awesome. :) Comment at: llvm/lib/Transforms/Scalar/SimplifyCFGPass.cpp:263 const SimplifyCFGOptions &Options) { - assert((!RequireAndPreserveDomTree || - (DT && DT

[PATCH] D94827: [SimplifyCFG] Require and preserve dominator tree

2021-01-15 Thread Jakub Kuderski via Phabricator via cfe-commits
kuhar added a comment. Wow, this is fantastic. When I first started working on the domtree updater back in 2017, SimplifyGFG seemed like one of the most difficult passes to handle, and I wasn't sure if we ever get there. Very impressive work, @lebedev.ri! Repository: rG LLVM Github Monorepo

[PATCH] D94827: [SimplifyCFG] Require and preserve dominator tree

2021-01-15 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri created this revision. lebedev.ri added reviewers: fhahn, jdoerfert, arsenm, mkazantsev, kuhar, brzycki, nikic. lebedev.ri added projects: LLVM, AMDGPU. Herald added subscribers: wenlei, kerbowa, steven_wu, hiraditya, nhaehnle, jvesely. lebedev.ri requested review of this revision. Her