[PATCH] D58700: [JumpThreading] Replace MergeBasicBlockIntoOnlyPred() with MergeBlockIntoPredecessor()

[Brian Rzycki via Phabricator via llvm-commits]

brzycki created this revision.
brzycki added reviewers: sebpop, asbirlea, chandlerc, davide, spatel.
Herald added subscribers: jdoerfert, jfb, zzheng, hiraditya.
Herald added a project: LLVM.

In D48202 <https://reviews.llvm.org/D48202> the helper function `MergeBasicBlockIntoOnlyPred()` was removed from llvm with the exception of JumpThreading. @asbirlea asked me to look at it for this final case.  This patch replaces the last remaining usage of  `MergeBasicBlockIntoOnlyPred()` with `MergeBlockIntoPredecessor()`. (I didn't fully remove the function and its test cases. I consider that work for a separate patch.)

This is a bit more complex than a simple function replacement for the following reasons:

1. `MergeBlockIntoPredecessor()` may return `false` and not perform the merge while `MergeBasicBlockIntoOnlyPred()` always performed the action.
2. The checks in `MergeBlockIntoPredecessor()` are slightly different.
3. `MergeBlockIntoPredecessor()` does block hoisting. This was by far the biggest change and caused several re-labeling of blocks in test cases.
4. In a few test cases the hoisting prevented existing checks from detecting trivially dead code from the existing logic in `ProcessBlock`.
5. A bug in `MergeBlockIntoPredecessor()` causing `DTU` asserts, fixed with D48202 <https://reviews.llvm.org/D48202>.

Passes `make-check` on x86_64, `test-suite` on x86_64. I see nominal gains of a few percent overall on compile_time in `test-suite` and `test-suite` with `CTmark`. The benchmark execution times are on par, indicating this change (should be) a no-op for emitted IR.

Note: This diff relies on D48202 <https://reviews.llvm.org/D48202> being comitted first. Without it `test-suite` benchmarks cause compiler asserts.

Finally, there is one instance in the test cases, `lvi-after-jumpthreading.ll` where hoisting causes the loss of valuable `LVI` information. There is no API to inform `LVI` two blocks have been merged other than deleting the merged block and (potentially) deleting the block that was merged into. It would be nice to have an LVI API similar to `LVI->threadEdge(...)` to prevent such metadata loss. Ideally it'd be a parameter to `MergeBlockIntoPredecessor()`.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D58700

Files:
  llvm/lib/Transforms/Scalar/JumpThreading.cpp
  llvm/test/Analysis/LazyValueAnalysis/lvi-after-jumpthreading.ll
  llvm/test/Transforms/CallSiteSplitting/callsite-split.ll
  llvm/test/Transforms/JumpThreading/PR33357-lvi-recursion.ll
  llvm/test/Transforms/JumpThreading/and-and-cond.ll
  llvm/test/Transforms/JumpThreading/and-cond.ll
  llvm/test/Transforms/JumpThreading/assume-edge-dom.ll
  llvm/test/Transforms/JumpThreading/basic.ll
  llvm/test/Transforms/JumpThreading/bb-unreachable-from-entry.ll
  llvm/test/Transforms/JumpThreading/combine-metadata.ll
  llvm/test/Transforms/JumpThreading/fold-not-thread.ll
  llvm/test/Transforms/JumpThreading/guards.ll
  llvm/test/Transforms/JumpThreading/header-succ.ll
  llvm/test/Transforms/JumpThreading/implied-cond.ll
  llvm/test/Transforms/JumpThreading/indirectbr.ll
  llvm/test/Transforms/JumpThreading/induction.ll
  llvm/test/Transforms/JumpThreading/lvi-tristate.ll
  llvm/test/Transforms/JumpThreading/phi-known.ll
  llvm/test/Transforms/JumpThreading/range-compare.ll
  llvm/test/Transforms/JumpThreading/removed-use.ll
  llvm/test/Transforms/JumpThreading/select.ll
  llvm/test/Transforms/JumpThreading/static-profile.ll
  llvm/test/Transforms/JumpThreading/thread-loads.ll
  llvm/test/Transforms/LoopUnroll/2011-08-09-PhiUpdate.ll
  llvm/test/Transforms/NewGVN/pair_jumpthread.ll
  llvm/test/Transforms/PhaseOrdering/simplifycfg-options.ll

-------------- next part --------------
A non-text attachment was scrubbed...
Name: D58700.188468.patch
Type: text/x-patch
Size: 34012 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20190226/720e2b39/attachment.bin>