[PATCH] D136237: [BasicBlockSections] avoid insertting redundant branch to fall through blocks

[Sinan Lin via Phabricator via llvm-commits]

sinan created this revision.
sinan added reviewers: rahmanl, tmsriram, efriedma.
Herald added subscribers: pengfei, hiraditya, kristof.beyls.
Herald added a project: All.
sinan requested review of this revision.
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

blocks reached by an explicit unconditional branch from its layout predecessor are also recognized as a fallthrough block via MachineBasicBlock::getFallThrough. For such cases, we do not need an extra unconditional branch.

If test case basic-block-sections_2.ll is run with O0, then you can see the instruction sequence like:

  __cxx_global_var_init:                  # @__cxx_global_var_init
  	.cfi_startproc
  # %bb.0:                                # %entry
  	pushq	%rax
  	.cfi_def_cfa_offset 16
  	movl	$5, %edi
  	callq	_Z3bari
  	movb	%al, %cl
  	xorl	%eax, %eax
                                          # kill: def $al killed $al killed $eax
  	testb	$1, %cl
  	movb	%al, 7(%rsp)                    # 1-byte Spill
  	jne	__cxx_global_var_init.__part.1
  	jmp	__cxx_global_var_init.__part.2
  	jmp	__cxx_global_var_init.__part.1
  .LBB_END0_0:
  	.cfi_endproc
  	.section	.text.startup,"ax", at progbits,unique,2
  __cxx_global_var_init.__part.1:         # %cond.true
  	.cfi_startproc
  	.cfi_def_cfa %rsp, 16
  	movl	$3, %edi
  	callq	_Z3bari

after this patch, the redundant branch `jmp	__cxx_global_var_init.__part.1` could be removed.

  __cxx_global_var_init:                  # @__cxx_global_var_init
  	.cfi_startproc
  # %bb.0:                                # %entry
  	pushq	%rax
  	.cfi_def_cfa_offset 16
  	movl	$5, %edi
  	callq	_Z3bari
  	movb	%al, %cl
  	xorl	%eax, %eax
                                          # kill: def $al killed $al killed $eax
  	testb	$1, %cl
  	movb	%al, 7(%rsp)                    # 1-byte Spill
  	jne	__cxx_global_var_init.__part.1
  	jmp	__cxx_global_var_init.__part.2
  .LBB_END0_0:
  	.cfi_endproc
  	.section	.text.startup,"ax", at progbits,unique,2
  __cxx_global_var_init.__part.1:         # %cond.true
  	.cfi_startproc
  	.cfi_def_cfa %rsp, 16
  	movl	$3, %edi
  	callq	_Z3bari

This issue happens much more frequently when I test bb section option with AArch64.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D136237

Files:
  llvm/lib/CodeGen/BasicBlockSections.cpp
  llvm/test/CodeGen/X86/basic-block-sections_2.ll


Index: llvm/test/CodeGen/X86/basic-block-sections_2.ll
===================================================================
--- llvm/test/CodeGen/X86/basic-block-sections_2.ll
+++ llvm/test/CodeGen/X86/basic-block-sections_2.ll
@@ -12,11 +12,18 @@
 ;;
 ; __cxx_global_var_init has multiple basic blocks which will produce many sections.
 ; RUN: llc < %s -mtriple=x86_64-pc-linux -function-sections -basic-block-sections=all -unique-basic-block-section-names | FileCheck %s
+; RUN: llc < %s -mtriple=x86_64-pc-linux -function-sections -basic-block-sections=all -O0 -unique-basic-block-section-names | FileCheck %s --check-prefix=REDUNDANT
 
 ; CHECK-LABEL: __cxx_global_var_init:
 ; CHECK-LABEL: __cxx_global_var_init.__part.1:
 ; CHECK-LABEL: __cxx_global_var_init.1:
 
+; REDUNDANT-LABEL: __cxx_global_var_init:
+; REDUNDANT: jmp __cxx_global_var_init.__part.2
+; REDUNDANT-NOT: jmp __cxx_global_var_init.__part.1
+; REDUNDANT-LABEL: __cxx_global_var_init.__part.1:
+; REDUNDANT-LABEL: __cxx_global_var_init.1:
+
 %class.A = type { i8 }
 
 $_ZN1AC2Eb = comdat any
Index: llvm/lib/CodeGen/BasicBlockSections.cpp
===================================================================
--- llvm/lib/CodeGen/BasicBlockSections.cpp
+++ llvm/lib/CodeGen/BasicBlockSections.cpp
@@ -128,6 +128,24 @@
                 "into clusters of basic blocks.",
                 false, false)
 
+/// Basic blocks with an explicit unconditional branch to its fallthrough block
+/// do not need an extra unconditional branch.
+static bool needInsertUncondBranch(const MachineBasicBlock &MBB) {
+  auto B = MBB.terminators().begin(), E = MBB.end(), I = B;
+  if (I == E)
+    return true;
+
+  if (I->isUnconditionalBranch())
+    return false;
+
+  while (I != E) {
+    if (I->isUnconditionalBranch())
+      return false;
+    I++;
+  }
+  return true;
+}
+
 // This function updates and optimizes the branching instructions of every basic
 // block in a given function to account for changes in the layout.
 static void updateBranches(
@@ -141,10 +159,13 @@
     // If this block had a fallthrough before we need an explicit unconditional
     // branch to that block if either
     //     1- the block ends a section, which means its next block may be
-    //        reorderd by the linker, or
+    //        reorderd by the linker,
     //     2- the fallthrough block is not adjacent to the block in the new
-    //        order.
-    if (FTMBB && (MBB.isEndSection() || &*NextMBBI != FTMBB))
+    //        order, or
+    //     3- the terminator of the block is an unconditional branch to its
+    //        fallthrough.
+    if (FTMBB && (MBB.isEndSection() || &*NextMBBI != FTMBB) &&
+        needInsertUncondBranch(MBB))
       TII->insertUnconditionalBranch(MBB, FTMBB, MBB.findBranchDebugLoc());
 
     // We do not optimize branches for machine basic blocks ending sections, as


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D136237.468818.patch
Type: text/x-patch
Size: 2867 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20221019/2d63e68e/attachment.bin>