[BranchFolding] Allow hoisting to block with a single conditional branch.

[gberry]

Summary

The BranchFolding pass is currently missing opportunities to hoist
common code if the hoisted-to block contains a single conditional branch
that has register uses. This occurs somewhat frequently on AArch64 with
CBZ/TBZ opcodes.

This change also eliminates some code differences when debug info is
present since the presence of e.g. DBG_VALUE instructions in the
hoisted-to block can enable hoisting that wouldn't have occurred without
them.

[gberry]

Ping?

[aprantl]

lib/CodeGen/BranchFolding.cpp

1919

Can you add a comment explaining what this condition checks for and why return Loc is appropriate here?

[gberry]

Add some comments

[kparzysz]

Looks ok to me.

[kparzysz]

One thought I had was: if there is a block with only an unconditional branch, should we fold it away first? That is, redirect any branches to that block to the target of the branch?

[gberry]

In D46324#1096882, @kparzysz wrote:

One thought I had was: if there is a block with only an unconditional branch, should we fold it away first? That is, redirect any branches to that block to the target of the branch?

I believe that should have already been taken care of (see OptimizeBlock() around line 1659). In the case that is being fixed by this patch the block contains just a *conditional* branch.

[kparzysz]

Alright then. LGTM.

lib/CodeGen/BranchFolding.cpp

if (MO.isUse()) {
       // If the terminator defines a register, make sure we don't hoist
       // the instruction whose def might be clobbered by the terminator.
       addRegAndItsAliases(Reg, TRI, Defs);
     }
   }
 
   if (Uses.empty())
     return Loc;
+  // If the terminator is the only instruction in the block and Uses is not
+  // empty (or we would have returned above), we can still safely hoist
+  // instructions just before the terminator as long as the Defs/Uses are not
+  // violated (which is checked in HoistCommonCodeInSuccs).
   if (Loc == MBB->begin())
-    return MBB->end();
+    return Loc;
 
   // The terminator is probably a conditional branch, try not to separate the
   // branch from condition setting instruction.
   MachineBasicBlock::iterator PI =
     skipDebugInstructionsBackward(std::prev(Loc), MBB->begin());
 
   bool IsDef = false;
   for (const MachineOperand &MO : PI->operands()) {

test/CodeGen/AArch64/branch-folder-oneinst.mir

+# RUN: llc -o - %s -mtriple=aarch64 -run-pass branch-folder | FileCheck %s
+# Check that BranchFolding pass is able to hoist a common instruction into a block with a single branch instruction.
+name: func
+tracksRegLiveness: true
+body: |
+  bb.0:
+    ; CHECK-LABEL: name: func
+    ; CHECK-LABEL: bb.0:
+    ; CHECK: $x0 = ADDXri $x0, 1, 0
+    ; CHECK: CBZX $x1, %bb.2
+    liveins: $x1
+    CBZX $x1, %bb.2
+
+  bb.1:
+    ; CHECK-LABEL: bb.1:
+    ; CHECK-NOT: $x0 = ADDXri $x0, 1, 0
+    liveins: $x0
+    $x0 = ADDXri $x0, 1, 0
+    $x0 = ADDXri $x0, 2, 0
+    RET_ReallyLR implicit $x0
+
+  bb.2:
+    ; CHECK-LABEL: bb.2:
+    ; CHECK-NOT: $x0 = ADDXri $x0, 1, 0
+    liveins: $x0
+    $x0 = ADDXri $x0, 1, 0
+    $x0 = ADDXri $x0, 3, 0
+    RET_ReallyLR implicit $x0
+...