[PATCH] D54411: [Codegen] Merge tail blocks after block placement

[Jim Lin via Phabricator via llvm-commits]

Jim created this revision.
Jim added a reviewer: haicheng.
Herald added subscribers: llvm-commits, PkmX.

I found the following case having tail blocks with no successors merging opportunities after block placement.

Before block placement:

bb0:

  ...
  bne a0, 0, bb2:

bb1:

  mv a0, 1
  ret 

bb2:

  ...

bb3:

  mv a0, 1
  ret

bb4:

  mv a0, -1
  ret

The conditional branch bne in bb0 is opposite to beq.

After block placement:

bb0:

  ...
  beq a0, 0, bb1

bb2:

  ...

bb4:

  mv a0, -1
  ret

bb1:

  mv a0, 1
  ret

bb3:

  mv a0, 1
  ret

After block placement, that appears new tail merging opportunity, bb1 and bb3 can be merged as one block. So the conditional constraint for merging tail blocks with no successors should be removed. In my experiment for RISC-V, it decreases code size.


Repository:
  rL LLVM

https://reviews.llvm.org/D54411

Files:
  lib/CodeGen/BranchFolding.cpp


Index: lib/CodeGen/BranchFolding.cpp
===================================================================
--- lib/CodeGen/BranchFolding.cpp
+++ lib/CodeGen/BranchFolding.cpp
@@ -1074,28 +1074,24 @@
   if (!EnableTailMerge) return MadeChange;
 
   // First find blocks with no successors.
-  // Block placement does not create new tail merging opportunities for these
-  // blocks.
-  if (!AfterBlockPlacement) {
-    MergePotentials.clear();
-    for (MachineBasicBlock &MBB : MF) {
-      if (MergePotentials.size() == TailMergeThreshold)
-        break;
-      if (!TriedMerging.count(&MBB) && MBB.succ_empty())
-        MergePotentials.push_back(MergePotentialsElt(HashEndOfMBB(MBB), &MBB));
-    }
-
-    // If this is a large problem, avoid visiting the same basic blocks
-    // multiple times.
+  MergePotentials.clear();
+  for (MachineBasicBlock &MBB : MF) {
     if (MergePotentials.size() == TailMergeThreshold)
-      for (unsigned i = 0, e = MergePotentials.size(); i != e; ++i)
-        TriedMerging.insert(MergePotentials[i].getBlock());
-
-    // See if we can do any tail merging on those.
-    if (MergePotentials.size() >= 2)
-      MadeChange |= TryTailMergeBlocks(nullptr, nullptr, MinCommonTailLength);
+      break;
+    if (!TriedMerging.count(&MBB) && MBB.succ_empty())
+      MergePotentials.push_back(MergePotentialsElt(HashEndOfMBB(MBB), &MBB));
   }
 
+  // If this is a large problem, avoid visiting the same basic blocks
+  // multiple times.
+  if (MergePotentials.size() == TailMergeThreshold)
+    for (unsigned i = 0, e = MergePotentials.size(); i != e; ++i)
+      TriedMerging.insert(MergePotentials[i].getBlock());
+
+  // See if we can do any tail merging on those.
+  if (MergePotentials.size() >= 2)
+    MadeChange |= TryTailMergeBlocks(nullptr, nullptr, MinCommonTailLength);
+
   // Look at blocks (IBB) with multiple predecessors (PBB).
   // We change each predecessor to a canonical form, by
   // (1) temporarily removing any unconditional branch from the predecessor


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D54411.173270.patch
Type: text/x-patch
Size: 2015 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20181112/d118ee81/attachment.bin>