Insertion of unconditional branch during FastISel differs when debug information is enabled

[tellenbach]

Summary

The insertion of an unconditional branch during FastISel can differ depending on building with or without debug information. This happens because void FastISel::fastEmitBranch emits an unconditional branch depending on the size of the current basic block without distinguishing between debug and non-debug instructions.

This patch fixes this issue by ignoring debug instructions when getting the size of the basic block.

llvm/trunk/include/llvm/IR/BasicBlock.h

public:
   instructionsWithoutDebug() const;
 
   /// Return an iterator range over the instructions in the block, skipping any
   /// debug instructions.
   iterator_range<filter_iterator<BasicBlock::iterator,
                                  std::function<bool(Instruction &)>>>
   instructionsWithoutDebug();
 
+  /// Return the size of the basic block ignoring debug instructions
+  filter_iterator<BasicBlock::const_iterator,
+                  std::function<bool(const Instruction &)>>::difference_type
+  sizeWithoutDebug() const;
+
   /// Unlink 'this' from the containing function, but do not delete it.
   void removeFromParent();
 
   /// Unlink 'this' from the containing function and delete it.
   ///
   // \returns an iterator pointing to the element after the erased one.
   SymbolTableList<BasicBlock>::iterator eraseFromParent();
 

llvm/trunk/lib/CodeGen/SelectionDAG/FastISel.cpp

bool FastISel::selectInstruction(const Instruction *I) {
   }
   return false;
 }
 
 /// Emit an unconditional branch to the given block, unless it is the immediate
 /// (fall-through) successor, and update the CFG.
 void FastISel::fastEmitBranch(MachineBasicBlock *MSucc,
                               const DebugLoc &DbgLoc) {
-  if (FuncInfo.MBB->getBasicBlock()->size() > 1 &&
+  if (FuncInfo.MBB->getBasicBlock()->sizeWithoutDebug() > 1 &&
       FuncInfo.MBB->isLayoutSuccessor(MSucc)) {
-    // For more accurate line information if this is the only instruction
-    // in the block then emit it, otherwise we have the unconditional
-    // fall-through case, which needs no instructions.
+    // For more accurate line information if this is the only non-debug
+    // instruction in the block then emit it, otherwise we have the
+    // unconditional fall-through case, which needs no instructions.
   } else {
     // The unconditional branch case.
     TII.insertBranch(*FuncInfo.MBB, MSucc, nullptr,
                      SmallVector<MachineOperand, 0>(), DbgLoc);
   }
   if (FuncInfo.BPI) {
     auto BranchProbability = FuncInfo.BPI->getEdgeProbability(
         FuncInfo.MBB->getBasicBlock(), MSucc->getBasicBlock());

llvm/trunk/lib/IR/BasicBlock.cpp

iterator_range<filter_iterator<BasicBlock::iterator,
                                std::function<bool(Instruction &)>>>
 BasicBlock::instructionsWithoutDebug() {
   std::function<bool(Instruction &)> Fn = [](Instruction &I) {
     return !isa<DbgInfoIntrinsic>(I);
   };
   return make_filter_range(*this, Fn);
 }
 
+filter_iterator<BasicBlock::const_iterator,
+                std::function<bool(const Instruction &)>>::difference_type
+BasicBlock::sizeWithoutDebug() const {
+  return std::distance(instructionsWithoutDebug().begin(),
+                       instructionsWithoutDebug().end());
+}
+
 void BasicBlock::removeFromParent() {
   getParent()->getBasicBlockList().remove(getIterator());
 }
 
 iplist<BasicBlock>::iterator BasicBlock::eraseFromParent() {
   return getParent()->getBasicBlockList().erase(getIterator());
 }
 

llvm/trunk/test/CodeGen/AArch64/fast-isel-branch-uncond-debug.ll

+; RUN: llc -mtriple=aarch64-arm-none-eabi -O1 -opt-bisect-limit=2 -o - %s  2> /dev/null | FileCheck %s
+
+define dso_local i32 @a() #0 !dbg !7 {
+entry:
+; CHECK:    b       .LBB0_1
+; CHECK:  .LBB0_1:
+  call void @llvm.dbg.value(metadata i32 0, metadata !12, metadata !DIExpression()), !dbg !13
+  br label %for.cond, !dbg !14
+
+; CHECK:    b       .LBB0_1
+; CHECK:  .Lfunc_end0:
+for.cond:
+  br label %for.cond, !dbg !15, !llvm.loop !18
+}
+declare void @llvm.lifetime.start.p0i8(i64 immarg, i8* nocapture) #1
+
+declare void @llvm.dbg.value(metadata, metadata, metadata) #2
+
+!llvm.dbg.cu = !{!0}
+!llvm.module.flags = !{!3, !4, !5}
+!llvm.ident = !{!6}
+
+!0 = distinct !DICompileUnit(language: DW_LANG_C99, file: !1, producer: "", isOptimized: true, runtimeVersion: 0, emissionKind: FullDebug, enums: !2, nameTableKind: None)
+!1 = !DIFile(filename: "fast-isel-branch-uncond-debug.ll", directory: "/tmp")
+!2 = !{}
+!3 = !{i32 2, !"Dwarf Version", i32 4}
+!4 = !{i32 2, !"Debug Info Version", i32 3}
+!5 = !{i32 1, !"wchar_size", i32 4}
+!6 = !{!""}
+!7 = distinct !DISubprogram(name: "a", scope: !1, file: !1, line: 1, type: !8, scopeLine: 1, spFlags: DISPFlagDefinition | DISPFlagOptimized, unit: !0, retainedNodes: !11)
+!8 = !DISubroutineType(types: !9)
+!9 = !{!10}
+!10 = !DIBasicType(name: "int", size: 32, encoding: DW_ATE_signed)
+!11 = !{!12}
+!12 = !DILocalVariable(name: "b", scope: !7, file: !1, line: 2, type: !10)
+!13 = !DILocation(line: 0, scope: !7)
+!14 = !DILocation(line: 3, column: 3, scope: !7)
+!15 = !DILocation(line: 3, column: 3, scope: !16)
+!16 = distinct !DILexicalBlock(scope: !17, file: !1, line: 3, column: 3)
+!17 = distinct !DILexicalBlock(scope: !7, file: !1, line: 3, column: 3)
+!18 = distinct !{!18, !19, !20}
+!19 = !DILocation(line: 3, column: 3, scope: !17)
+!20 = !DILocation(line: 4, column: 5, scope: !17)
+ 
 No newline at end of file

llvm/trunk/unittests/IR/BasicBlockTest.cpp

TEST(BasicBlockTest, TestInstructionsWithoutDebug) {
   Builder1.CreateCall(DbgDeclare, Args);
   Instruction *SubInst = cast<Instruction>(Builder1.CreateSub(MulInst, V));
   Builder1.CreateCall(DbgAddr, Args);
 
   SmallVector<Instruction *, 4> Exp = {Var, AddInst, MulInst, SubInst};
   CHECK_ITERATORS(BB1->instructionsWithoutDebug(), Exp);
   CHECK_ITERATORS(BBConst->instructionsWithoutDebug(), Exp);
 
+  EXPECT_EQ(static_cast<size_t>(BB1->sizeWithoutDebug()), Exp.size());
+  EXPECT_EQ(static_cast<size_t>(BBConst->sizeWithoutDebug()), Exp.size());
+
   delete M;
   delete V;
 }
 
 } // End anonymous namespace.
 } // End llvm namespace.