[PATCH] D105238: GlobalISel/AArch64: don't optimize away redundant branches at -O0

[Adrian Prantl via Phabricator via llvm-commits]

aprantl created this revision.
aprantl added reviewers: aemerson, paquette, vsk, t.p.northover.
Herald added subscribers: danielkiss, hiraditya, kristof.beyls, rovka.
aprantl requested review of this revision.
Herald added a project: LLVM.

This patch prevents GlobalISel from optimizing out redundant branch instructions when compiling without optimizations.

The motivating example is code like the following common pattern in Swift, where users expect to be able to set a breakpoint on the early exit:

  public func f(b: Bool) {
    guard b else { 
      return // I would like to set a breakpoint here.
    }
    ...
  }

The patch modifies two places in GlobalISEL: The first one is in IRTranslator.cpp where the removal of redundant branches is made conditional on the optimization level. The second one is in AArch64InstructionSelector.cpp where an -O0 *only* optimization is being removed.

rdar://79515454


https://reviews.llvm.org/D105238

Files:
  llvm/lib/CodeGen/GlobalISel/IRTranslator.cpp
  llvm/lib/Target/AArch64/GISel/AArch64InstructionSelector.cpp
  llvm/test/DebugInfo/AArch64/fallthrough-branch.ll


Index: llvm/test/DebugInfo/AArch64/fallthrough-branch.ll
===================================================================
--- /dev/null
+++ llvm/test/DebugInfo/AArch64/fallthrough-branch.ll
@@ -0,0 +1,52 @@
+; RUN: llc -O0 -stop-before=livedebugvalues < %s | FileCheck %s
+
+; ModuleID = '/tmp/t.o'
+source_filename = "/tmp/t.o"
+target datalayout = "e-m:o-i64:64-i128:128-n32:64-S128"
+target triple = "arm64-apple-macosx11.0.0"
+
+ at __swift_reflection_version = linkonce_odr hidden constant i16 3
+ at llvm.swift_module_hash = private constant [16 x i8] c"\AD\85\B7\B6\00H\8F\19\17\F8\91V\14\0A\1F*", section "__LLVM,__swift_modhash"
+ at llvm.used = appending global [3 x i8*] [i8* bitcast (void (i1)* @"$s1t1f1bySb_tF" to i8*), i8* bitcast (i16* @__swift_reflection_version to i8*), i8* getelementptr inbounds ([16 x i8], [16 x i8]* @llvm.swift_module_hash, i32 0, i32 0)], section "llvm.metadata", align 8
+
+define swiftcc void @"$s1t1f1bySb_tF"(i1 %0) !dbg !35 {
+  %2 = alloca i1, align 8
+  %3 = bitcast i1* %2 to i8*
+  call void @llvm.memset.p0i8.i64(i8* align 8 %3, i8 0, i64 1, i1 false)
+  store i1 %0, i1* %2, align 8, !dbg !37
+; CHECK:   B %bb.1, debug-location !{{[0-9]+}}
+  br i1 %0, label %4, label %5, !dbg !38
+
+4:                                                ; preds = %1
+; Check that at -O0 the branches and their debug locations are not eliminated.
+; CHECK:   B %bb.3, debug-location !{{[0-9]+}}
+  br label %6, !dbg !39
+
+5:                                                ; preds = %1
+; CHECK:   B %bb.3, debug-location !{{[0-9]+}}
+  br label %6, !dbg !40
+
+6:                                                ; preds = %4, %5
+  ret void, !dbg !39
+}
+
+; Function Attrs: argmemonly nofree nosync nounwind willreturn writeonly
+declare void @llvm.memset.p0i8.i64(i8* nocapture writeonly, i8, i64, i1 immarg) #1
+attributes #1 = { argmemonly nofree nosync nounwind willreturn writeonly }
+
+!llvm.module.flags = !{!6, !7, !14}
+!llvm.dbg.cu = !{!15, !27}
+
+!6 = !{i32 7, !"Dwarf Version", i32 4}
+!7 = !{i32 2, !"Debug Info Version", i32 3}
+!14 = !{i32 1, !"Swift Version", i32 7}
+!15 = distinct !DICompileUnit(language: DW_LANG_Swift, file: !16, producer: "Swift", emissionKind: LineTablesOnly)
+!16 = !DIFile(filename: "t.swift", directory: "/tmp")
+!17 = !{}
+!27 = distinct !DICompileUnit(language: DW_LANG_ObjC, file: !16, emissionKind: LineTablesOnly)
+!35 = distinct !DISubprogram(name: "f", linkageName: "$s1t1f1bySb_tF", scope: !15, file: !16, line: 1, type: !36, scopeLine: 1, spFlags: DISPFlagDefinition, unit: !15, retainedNodes: !17)
+!36 = !DISubroutineType(types: null)
+!37 = !DILocation(line: 0, scope: !35)
+!38 = !DILocation(line: 2, column: 9, scope: !35)
+!39 = !DILocation(line: 3, column: 1, scope: !35)
+!40 = !DILocation(line: 2, column: 18, scope: !35)
Index: llvm/lib/Target/AArch64/GISel/AArch64InstructionSelector.cpp
===================================================================
--- llvm/lib/Target/AArch64/GISel/AArch64InstructionSelector.cpp
+++ llvm/lib/Target/AArch64/GISel/AArch64InstructionSelector.cpp
@@ -2093,17 +2093,8 @@
     I.eraseFromParent();
     return true;
   }
-  case TargetOpcode::G_BR: {
-    // If the branch jumps to the fallthrough block, don't bother emitting it.
-    // Only do this for -O0 for a good code size improvement, because when
-    // optimizations are enabled we want to leave this choice to
-    // MachineBlockPlacement.
-    bool EnableOpt = MF.getTarget().getOptLevel() != CodeGenOpt::None;
-    if (EnableOpt || !MBB.isLayoutSuccessor(I.getOperand(0).getMBB()))
+  case TargetOpcode::G_BR:
       return false;
-    I.eraseFromParent();
-    return true;
-  }
   case TargetOpcode::G_SHL:
     return earlySelectSHL(I, MRI);
   case TargetOpcode::G_CONSTANT: {
Index: llvm/lib/CodeGen/GlobalISel/IRTranslator.cpp
===================================================================
--- llvm/lib/CodeGen/GlobalISel/IRTranslator.cpp
+++ llvm/lib/CodeGen/GlobalISel/IRTranslator.cpp
@@ -566,7 +566,7 @@
 
   if (BrInst.isUnconditional()) {
     // If the unconditional target is the layout successor, fallthrough.
-    if (!CurMBB.isLayoutSuccessor(Succ0MBB))
+    if (OptLevel == CodeGenOpt::None || !CurMBB.isLayoutSuccessor(Succ0MBB))
       MIRBuilder.buildBr(*Succ0MBB);
 
     // Link successors.


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D105238.355704.patch
Type: text/x-patch
Size: 4314 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20210630/65dd456f/attachment.bin>