[PATCH] D48202: Generalize MergeBlockIntoPredecessor. Replace uses of MergeBasicBlockIntoOnlyPred.

[Alina Sbirlea via Phabricator via llvm-commits]

asbirlea created this revision.
asbirlea added reviewers: chandlerc, spatel, davide, brzycki, bkramer.
Herald added subscribers: kbarton, tpr, jlebar, nhaehnle, nemanjai, sanjoy, qcolombet.
Herald added a reviewer: javed.absar.

Two utils methods have essentially the same functionality. This is an attempt to merge them into one.

1. lib/Transforms/Utils/Local.cpp : MergeBasicBlockIntoOnlyPred
2. lib/Transforms/Utils/BasicBlockUtils.cpp : MergeBlockIntoPredecessor

Prior to the patch:

1. MergeBasicBlockIntoOnlyPred

Updates either DomTree or DeferredDominance
Moves all instructions from Pred to BB, deletes Pred
Asserts BB has single predecessor
If address was taken, replace the block address with constant 1 (?)

2. MergeBlockIntoPredecessor

Updates DomTree, LoopInfo and MemoryDependenceResults
Moves all instruction from BB to Pred, deletes BB
Returns if doesn't have a single predecessor
Returns if BB's address was taken

After the patch:
Method 2. MergeBlockIntoPredecessor is attempting to become the new default:
Updates DomTree or DeferredDominance, and LoopInfo and MemoryDependenceResults
Moves all instruction from BB to Pred, deletes BB
Returns if doesn't have a single predecessor
Returns if BB's address was taken

Uses of MergeBasicBlockIntoOnlyPred that need to be replaced:

1. lib/Transforms/Scalar/LoopSimplifyCFG.cpp

Updated in this patch. No challenges.

2. lib/CodeGen/CodeGenPrepare.cpp

Updated in this patch.

  i. eliminateFallThrough is straightforward, but I added using a temporary array to avoid the iterator invalidation.
  ii. eliminateMostlyEmptyBlock(s) methods also now use a temporary array for blocks

Some interesting aspects:

- Since Pred is not deleted (BB is), the entry block does not need updating.
- The entry block was being updated with the deleted block in eliminateMostlyEmptyBlock. Added assert to make obvious that BB=SinglePred.
- isMergingEmptyBlockProfitable assumes BB is the one to be deleted.
- eliminateMostlyEmptyBlock(BB) does not delete BB on one path, it deletes its unique predecessor instead.
- adding some test owner as subscribers for the interesting tests modified: test/CodeGen/X86/avx-cmp.ll test/CodeGen/AMDGPU/nested-loop-conditions.ll test/CodeGen/AMDGPU/si-annotate-cf.ll test/CodeGen/X86/hoist-spill.ll test/CodeGen/X86/2006-11-17-IllegalMove.ll

3. lib/Transforms/Scalar/JumpThreading.cpp

Not covered in this patch. It is the only use case using the DeferredDominance.
I would defer to Brian Rzycki to make this replacement.


Repository:
  rL LLVM

https://reviews.llvm.org/D48202

Files:
  include/llvm/Transforms/Utils/BasicBlockUtils.h
  lib/CodeGen/CodeGenPrepare.cpp
  lib/Transforms/Scalar/LoopSimplifyCFG.cpp
  lib/Transforms/Utils/BasicBlockUtils.cpp
  test/CodeGen/AMDGPU/branch-relaxation.ll
  test/CodeGen/AMDGPU/nested-loop-conditions.ll
  test/CodeGen/AMDGPU/si-annotate-cf.ll
  test/CodeGen/ARM/indirectbr.ll
  test/CodeGen/PowerPC/memCmpUsedInZeroEqualityComparison.ll
  test/CodeGen/PowerPC/memcmp-mergeexpand.ll
  test/CodeGen/PowerPC/ppc-shrink-wrapping.ll
  test/CodeGen/PowerPC/simplifyConstCmpToISEL.ll
  test/CodeGen/Thumb2/2010-11-22-EpilogueBug.ll
  test/CodeGen/Thumb2/thumb2-jtb.ll
  test/CodeGen/X86/2006-11-17-IllegalMove.ll
  test/CodeGen/X86/avx-cmp.ll
  test/CodeGen/X86/avx-splat.ll
  test/CodeGen/X86/avx2-vbroadcast.ll
  test/CodeGen/X86/avx512-i1test.ll
  test/CodeGen/X86/block-placement.ll
  test/CodeGen/X86/hoist-spill.ll
  test/CodeGen/X86/ins_subreg_coalesce-1.ll
  test/CodeGen/X86/memcmp-mergeexpand.ll
  test/CodeGen/X86/pr32108.ll
  test/CodeGen/X86/setcc-lowering.ll
  test/CodeGen/X86/split-store.ll
  test/CodeGen/X86/tail-dup-merge-loop-headers.ll
  test/DebugInfo/Generic/sunk-compare.ll
  test/Transforms/CodeGenPrepare/X86/computedgoto.ll
  test/Transforms/CodeGenPrepare/basic.ll
  test/Transforms/LoopStrengthReduce/ARM/2012-06-15-lsr-noaddrmode.ll
  test/Transforms/LoopStrengthReduce/X86/2012-01-13-phielim.ll
  test/Transforms/SimpleLoopUnswitch/trivial-unswitch-iteration.ll

-------------- next part --------------
A non-text attachment was scrubbed...
Name: D48202.151439.patch
Type: text/x-patch
Size: 26194 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20180615/9dfb074e/attachment-0001.bin>