[AArch64] Teach analyzeBranch to remove branch equivelent to fallthrough

[reames]

Summary

The motivation here is that MachineBlockPlacement relies on analyzeBranch to remove branches to fallthrough blocks when the branch is not fully analyzeable. With the introduction of the FAULTING_OP psuedo for implicit null checking (see D87861), this case becomes important. Note that it's hard to otherwise exercise this path as BranchFolding handle's any fully analyzeable branch sequence without using this interface.

p.s. For anyone who saw my comment in the original review, what I thought was an issue in BranchFolding originally turned out to simply be a bug in my patch. (Now fixed.)

[resistor]

llvm/lib/Target/AArch64/AArch64InstrInfo.cpp

307

What's the situation where this is needed?

[reames]

llvm/lib/Target/AArch64/AArch64InstrInfo.cpp

307

Looking at it, that one should be dead. I'll convert to an assert to confirm.

The conditional branch case below is definitely not dead.

[reames]

address review comment

[resistor]

LGTM

llvm/lib/Target/AArch64/AArch64InstrInfo.cpp

307

I feel like this assert message could have more context.

llvm/lib/Target/AArch64/AArch64InstrInfo.cpp

while (isUncondBranchOpcode(SecondLastOpc)) {
         return false;
       } else {
         SecondLastInst = &*I;
         SecondLastOpc = SecondLastInst->getOpcode();
       }
     }
   }
 
+  // If we're allowed to modify and the block ends in a unconditional branch
+  // which could simply fallthrough, remove the branch.  (Note: This case only
+  // matters when we can't understand the whole sequence, otherwise it's also
+  // handled by BranchFolding.cpp.)
+  if (AllowModify && isUncondBranchOpcode(LastOpc) &&
+      MBB.isLayoutSuccessor(getBranchDestBlock(*LastInst))) {
+    LastInst->eraseFromParent();
+    LastInst = SecondLastInst;
+    LastOpc = LastInst->getOpcode();
+    if (I == MBB.begin() || !isUnpredicatedTerminator(*--I)) {
+      assert(!isUncondBranchOpcode(LastOpc) &&
+             "unreachable unconditional branches removed above");
+
+      if (isCondBranchOpcode(LastOpc)) {
+        // Block ends with fall-through condbranch.
+        parseCondBranch(LastInst, TBB, Cond);
+        return false;
+      }
+      return true; // Can't handle indirect branch.
+    } else {
+      SecondLastInst = &*I;
+      SecondLastOpc = SecondLastInst->getOpcode();
+    }
+  }
+
   // If there are three terminators, we don't know what sort of block this is.
   if (SecondLastInst && I != MBB.begin() && isUnpredicatedTerminator(*--I))
     return true;
 
   // If the block ends with a B and a Bcc, handle it.
   if (isCondBranchOpcode(SecondLastOpc) && isUncondBranchOpcode(LastOpc)) {
     parseCondBranch(SecondLastInst, TBB, Cond);
     FBB = LastInst->getOperand(0).getMBB();

llvm/test/CodeGen/AArch64/implicit-null-check.ll

 ; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
 ; RUN: llc < %s -verify-machineinstrs -O3 -mtriple=aarch64-unknown-unknown -enable-implicit-null-checks | FileCheck %s
 
 ; Basic test for implicit null check conversion - this is analogous to the
 ; file with the same name in the X86 tree, but adjusted to remove patterns
 ; related to memory folding of arithmetic (since aarch64 doesn't), and add
 ; a couple of aarch64 specific tests.
 
 define i32 @imp_null_check_load_fallthrough(i32* %x) {
 ; CHECK-LABEL: imp_null_check_load_fallthrough:
 ; CHECK:       // %bb.0: // %entry
 ; CHECK-NEXT:  .Ltmp0:
 ; CHECK-NEXT:    ldr w0, [x0] // on-fault: .LBB0_2
-; CHECK-NEXT:    b .LBB0_1
-; CHECK-NEXT:  .LBB0_1: // %not_null
+; CHECK-NEXT:  // %bb.1: // %not_null
 ; CHECK-NEXT:    ret
 ; CHECK-NEXT:  .LBB0_2: // %is_null
 ; CHECK-NEXT:    mov w0, #42
 ; CHECK-NEXT:    ret
  entry:
   %c = icmp eq i32* %x, null
   br i1 %c, label %is_null, label %not_null, !make.implicit !0
 
  not_null:
   %t = load i32, i32* %x
   ret i32 %t
 
 is_null:
   ret i32 42
 }
 
 
 define i32 @imp_null_check_load_reorder(i32* %x) {
 ; CHECK-LABEL: imp_null_check_load_reorder:
 ; CHECK:       // %bb.0: // %entry
 ; CHECK-NEXT:  .Ltmp1:
 ; CHECK-NEXT:    ldr w0, [x0] // on-fault: .LBB1_2
-; CHECK-NEXT:    b .LBB1_1
-; CHECK-NEXT:  .LBB1_1: // %not_null
+; CHECK-NEXT:  // %bb.1: // %not_null
 ; CHECK-NEXT:    ret
 ; CHECK-NEXT:  .LBB1_2: // %is_null
 ; CHECK-NEXT:    mov w0, #42
 ; CHECK-NEXT:    ret
  entry:
   %c = icmp eq i32* %x, null
   br i1 %c, label %is_null, label %not_null, !make.implicit !0
 
  is_null:
   ret i32 42
 
  not_null:
   %t = load i32, i32* %x
   ret i32 %t
 }
 
 define i32 @imp_null_check_unordered_load(i32* %x) {
 ; CHECK-LABEL: imp_null_check_unordered_load:
 ; CHECK:       // %bb.0: // %entry
 ; CHECK-NEXT:  .Ltmp2:
 ; CHECK-NEXT:    ldr w0, [x0] // on-fault: .LBB2_2
-; CHECK-NEXT:    b .LBB2_1
-; CHECK-NEXT:  .LBB2_1: // %not_null
+; CHECK-NEXT:  // %bb.1: // %not_null
 ; CHECK-NEXT:    ret
 ; CHECK-NEXT:  .LBB2_2: // %is_null
 ; CHECK-NEXT:    mov w0, #42
 ; CHECK-NEXT:    ret
  entry:
   %c = icmp eq i32* %x, null
   br i1 %c, label %is_null, label %not_null, !make.implicit !0
 

 }
 
 
 define i8 @imp_null_check_load_i8(i8* %x) {
 ; CHECK-LABEL: imp_null_check_load_i8:
 ; CHECK:       // %bb.0: // %entry
 ; CHECK-NEXT:  .Ltmp3:
 ; CHECK-NEXT:    ldrb w0, [x0] // on-fault: .LBB5_2
-; CHECK-NEXT:    b .LBB5_1
-; CHECK-NEXT:  .LBB5_1: // %not_null
+; CHECK-NEXT:  // %bb.1: // %not_null
 ; CHECK-NEXT:    ret
 ; CHECK-NEXT:  .LBB5_2: // %is_null
 ; CHECK-NEXT:    mov w0, #42
 ; CHECK-NEXT:    ret
  entry:
   %c = icmp eq i8* %x, null
   br i1 %c, label %is_null, label %not_null, !make.implicit !0
 

 
 
 
 define i32 @imp_null_check_gep_load(i32* %x) {
 ; CHECK-LABEL: imp_null_check_gep_load:
 ; CHECK:       // %bb.0: // %entry
 ; CHECK-NEXT:  .Ltmp4:
 ; CHECK-NEXT:    ldr w0, [x0, #128] // on-fault: .LBB7_2
-; CHECK-NEXT:    b .LBB7_1
-; CHECK-NEXT:  .LBB7_1: // %not_null
+; CHECK-NEXT:  // %bb.1: // %not_null
 ; CHECK-NEXT:    ret
 ; CHECK-NEXT:  .LBB7_2: // %is_null
 ; CHECK-NEXT:    mov w0, #42
 ; CHECK-NEXT:    ret
  entry:
   %c = icmp eq i32* %x, null
   br i1 %c, label %is_null, label %not_null, !make.implicit !0
 
  is_null:
   ret i32 42
 
  not_null:
   %x.gep = getelementptr i32, i32* %x, i32 32
   %t = load i32, i32* %x.gep
   ret i32 %t
 }
 
 define i32 @imp_null_check_add_result(i32* %x, i32 %p) {
 ; CHECK-LABEL: imp_null_check_add_result:
 ; CHECK:       // %bb.0: // %entry
 ; CHECK-NEXT:  .Ltmp5:
 ; CHECK-NEXT:    ldr w8, [x0] // on-fault: .LBB8_2
-; CHECK-NEXT:    b .LBB8_1
-; CHECK-NEXT:  .LBB8_1: // %not_null
+; CHECK-NEXT:  // %bb.1: // %not_null
 ; CHECK-NEXT:    add w0, w8, w1
 ; CHECK-NEXT:    ret
 ; CHECK-NEXT:  .LBB8_2: // %is_null
 ; CHECK-NEXT:    mov w0, #42
 ; CHECK-NEXT:    ret
  entry:
   %c = icmp eq i32* %x, null
   br i1 %c, label %is_null, label %not_null, !make.implicit !0

 
 ; Can hoist over a potential faulting instruction as long as we don't
 ; change the conditions under which the instruction faults.
 define i32 @imp_null_check_hoist_over_udiv(i32* %x, i32 %a, i32 %b) {
 ; CHECK-LABEL: imp_null_check_hoist_over_udiv:
 ; CHECK:       // %bb.0: // %entry
 ; CHECK-NEXT:  .Ltmp6:
 ; CHECK-NEXT:    ldr w8, [x0] // on-fault: .LBB9_2
-; CHECK-NEXT:    b .LBB9_1
-; CHECK-NEXT:  .LBB9_1: // %not_null
+; CHECK-NEXT:  // %bb.1: // %not_null
 ; CHECK-NEXT:    udiv w9, w1, w2
 ; CHECK-NEXT:    add w0, w8, w9
 ; CHECK-NEXT:    ret
 ; CHECK-NEXT:  .LBB9_2: // %is_null
 ; CHECK-NEXT:    mov w0, #42
 ; CHECK-NEXT:    ret
  entry:
   %c = icmp eq i32* %x, null
not_null:
   ret i32 %t1
 }
 
 define i32 @imp_null_check_gep_load_with_use_dep(i32* %x, i32 %a) {
 ; CHECK-LABEL: imp_null_check_gep_load_with_use_dep:
 ; CHECK:       // %bb.0: // %entry
 ; CHECK-NEXT:  .Ltmp7:
 ; CHECK-NEXT:    ldr w8, [x0] // on-fault: .LBB11_2
-; CHECK-NEXT:    b .LBB11_1
-; CHECK-NEXT:  .LBB11_1: // %not_null
+; CHECK-NEXT:  // %bb.1: // %not_null
 ; CHECK-NEXT:    add w9, w0, w1
 ; CHECK-NEXT:    add w8, w9, w8
 ; CHECK-NEXT:    add w0, w8, #4 // =4
 ; CHECK-NEXT:    ret
 ; CHECK-NEXT:  .LBB11_2: // %is_null
 ; CHECK-NEXT:    mov w0, #42
 ; CHECK-NEXT:    ret
  entry:
not_null:
   ret void
 }
 
 define i32 @imp_null_check_neg_gep_load(i32* %x) {
 ; CHECK-LABEL: imp_null_check_neg_gep_load:
 ; CHECK:       // %bb.0: // %entry
 ; CHECK-NEXT:  .Ltmp8:
 ; CHECK-NEXT:    ldur w0, [x0, #-128] // on-fault: .LBB16_2
-; CHECK-NEXT:    b .LBB16_1
-; CHECK-NEXT:  .LBB16_1: // %not_null
+; CHECK-NEXT:  // %bb.1: // %not_null
 ; CHECK-NEXT:    ret
 ; CHECK-NEXT:  .LBB16_2: // %is_null
 ; CHECK-NEXT:    mov w0, #42
 ; CHECK-NEXT:    ret
  entry:
   %c = icmp eq i32* %x, null
   br i1 %c, label %is_null, label %not_null, !make.implicit !0