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

[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.
Herald added subscribers: cfe-commits, wdng.
Herald added a project: clang.

My motivation is the following two problems i've hit myself:

- `FoldBranchToCommonDest()` (and possibly `FoldValueComparisonIntoPredecessors()`) can't 'really' be extended to handle the case where bonus instruction has external uses, in the case where we have a successor that has multiple predecessors, but is dominated by the def block - there may not be a PHI node, we need to form it, but to know how to fill it we need to check for dominance, but well, we don't have the Dominator tree :) While it would be possible to write a function to check that some block dominates another, it seems like a hack. (59560e85897afc50090b6c3d920bacfd28b49d06 <https://reviews.llvm.org/rG59560e85897afc50090b6c3d920bacfd28b49d06>, https://bugs.llvm.org/show_bug.cgi?id=48450)
- Even though SimplifyCFGPass drops unreachable blocks (`removeUnreachableBlocks()`), SimplifyCFG transformations may end up creating more unreachable blocks, and we may happily try to SimplifyCFG them, which is, at best, not very profitable, and at worst leads to crashes (ef93f7a11c347534ac768ec8bbbed64cd20c41d2 <https://reviews.llvm.org/rGef93f7a11c347534ac768ec8bbbed64cd20c41d2>, https://bugs.llvm.org/show_bug.cgi?id=48450#c8)

Indeed, simplifycfg pass didn't know how to preserve dominator tree,
it took me just under a month (starting with e1133179587dd895962a2fe4d6eb0cb1e63b5ee2 <https://reviews.llvm.org/rGe1133179587dd895962a2fe4d6eb0cb1e63b5ee2>)
do rectify that, now it fully knows how to, there's likely some problems with that still,
but i've dealt with everything i can spot so far.

I think we now can flip the switch.

Full disclosure: this isn't a compile-time win per-se, nor do i expect it to be one
should we actually start to make use of DominatorTree in SimplifyCFG.
Current best-case results suggest +0.5% geomean compile-time impact:
https://llvm-compile-time-tracker.com/compare.php?from=a14c36fe27f5c36de44049237011d140a7277774&to=fd53c482aca9e20d76fb57def3ab0af851d1c697&stat=instructions


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D94827

Files:
  clang/test/CodeGen/thinlto-distributed-newpm.ll
  llvm/lib/CodeGen/DwarfEHPrepare.cpp
  llvm/lib/Target/AMDGPU/AMDGPUUnifyDivergentExitNodes.cpp
  llvm/lib/Transforms/Scalar/SimplifyCFGPass.cpp
  llvm/lib/Transforms/Utils/SimplifyCFG.cpp
  llvm/test/CodeGen/AArch64/O3-pipeline.ll
  llvm/test/CodeGen/AMDGPU/opt-pipeline.ll
  llvm/test/CodeGen/ARM/O3-pipeline.ll
  llvm/test/Other/new-pm-defaults.ll
  llvm/test/Other/new-pm-thinlto-defaults.ll
  llvm/test/Other/new-pm-thinlto-postlink-pgo-defaults.ll
  llvm/test/Other/new-pm-thinlto-postlink-samplepgo-defaults.ll
  llvm/test/Other/new-pm-thinlto-prelink-pgo-defaults.ll
  llvm/test/Other/new-pm-thinlto-prelink-samplepgo-defaults.ll
  llvm/test/Other/opt-LTO-pipeline.ll
  llvm/test/Other/opt-O2-pipeline.ll
  llvm/test/Other/opt-O3-pipeline-enable-matrix.ll
  llvm/test/Other/opt-O3-pipeline.ll
  llvm/test/Other/opt-Os-pipeline.ll
  llvm/test/Other/pm-pgo-preinline.ll

-------------- next part --------------
A non-text attachment was scrubbed...
Name: D94827.317069.patch
Type: text/x-patch
Size: 28734 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20210115/765ec4c4/attachment-0001.bin>