Explicitly check for entry basic block, rather than relying on MachineBasicBlock::pred_empty.

Sometimes in unoptimized code, we have dangling unreachable basic blocks with no predecessors. Basic block sections should be emitted for those as well. Without this patch, the included test fails with a fatal error in `AsmPrinter::emitBasicBlockEnd`.

Reviewed By: tmsriram

Differential Revision: https://reviews.llvm.org/D89423

Author: rlavaee

llvm/include/llvm/CodeGen/AsmPrinter.h

@@ -778,6 +778,9 @@ class AsmPrinter : public MachineFunctionPass {
   GCMetadataPrinter *GetOrCreateGCPrinter(GCStrategy &S);
   /// Emit GlobalAlias or GlobalIFunc.
   void emitGlobalIndirectSymbol(Module &M, const GlobalIndirectSymbol &GIS);
+
+  /// This method decides whether the specified basic block requires a label.
+  bool shouldEmitLabelForBasicBlock(const MachineBasicBlock &MBB) const;
 };
 
 } // end namespace llvm

llvm/include/llvm/CodeGen/MachineBasicBlock.h

@@ -434,6 +434,9 @@ class MachineBasicBlock
 
   bool hasEHPadSuccessor() const;
 
+  /// Returns true if this is the entry block of the function.
+  bool isEntryBlock() const;
+
   /// Returns true if this is the entry block of an EH scope, i.e., the block
   /// that used to have a catchpad or cleanuppad instruction in the LLVM IR.
   bool isEHScopeEntry() const { return IsEHScopeEntry; }

llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp

@@ -1053,7 +1053,7 @@ void AsmPrinter::emitBBAddrMapSection(const MachineFunction &MF) {
   // Emit BB Information for each basic block in the funciton.
   for (const MachineBasicBlock &MBB : MF) {
     const MCSymbol *MBBSymbol =
-        MBB.pred_empty() ? FunctionSymbol : MBB.getSymbol();
+        MBB.isEntryBlock() ? FunctionSymbol : MBB.getSymbol();
     // Emit the basic block offset.
     emitLabelDifferenceAsULEB128(MBBSymbol, FunctionSymbol);
     // Emit the basic block size. When BBs have alignments, their size cannot
@@ -3088,7 +3088,7 @@ void AsmPrinter::emitBasicBlockStart(const MachineBasicBlock &MBB) {
   // Switch to a new section if this basic block must begin a section. The
   // entry block is always placed in the function section and is handled
   // separately.
-  if (MBB.isBeginSection() && !MBB.pred_empty()) {
+  if (MBB.isBeginSection() && !MBB.isEntryBlock()) {
     OutStreamer->SwitchSection(
         getObjFileLowering().getSectionForMachineBasicBlock(MF->getFunction(),
                                                             MBB, TM));
@@ -3126,24 +3126,22 @@ void AsmPrinter::emitBasicBlockStart(const MachineBasicBlock &MBB) {
   }
 
   // Print the main label for the block.
-  if (MBB.pred_empty() ||
-      (!MF->hasBBLabels() && isBlockOnlyReachableByFallthrough(&MBB) &&
-       !MBB.isEHFuncletEntry() && !MBB.hasLabelMustBeEmitted())) {
+  if (shouldEmitLabelForBasicBlock(MBB)) {
+    if (isVerbose() && MBB.hasLabelMustBeEmitted())
+      OutStreamer->AddComment("Label of block must be emitted");
+    OutStreamer->emitLabel(MBB.getSymbol());
+  } else {
     if (isVerbose()) {
       // NOTE: Want this comment at start of line, don't emit with AddComment.
       OutStreamer->emitRawComment(" %bb." + Twine(MBB.getNumber()) + ":",
                                   false);
     }
-  } else {
-    if (isVerbose() && MBB.hasLabelMustBeEmitted())
-      OutStreamer->AddComment("Label of block must be emitted");
-    OutStreamer->emitLabel(MBB.getSymbol());
   }
 
   // With BB sections, each basic block must handle CFI information on its own
   // if it begins a section (Entry block is handled separately by
   // AsmPrinterHandler::beginFunction).
-  if (MBB.isBeginSection() && !MBB.pred_empty())
+  if (MBB.isBeginSection() && !MBB.isEntryBlock())
     for (const HandlerInfo &HI : Handlers)
       HI.Handler->beginBasicBlock(MBB);
 }
@@ -3177,15 +3175,26 @@ void AsmPrinter::emitVisibility(MCSymbol *Sym, unsigned Visibility,
     OutStreamer->emitSymbolAttribute(Sym, Attr);
 }
 
+bool AsmPrinter::shouldEmitLabelForBasicBlock(
+    const MachineBasicBlock &MBB) const {
+  // With `-fbasic-block-sections=`, a label is needed for every non-entry block
+  // in the labels mode (option `=labels`) and every section beginning in the
+  // sections mode (`=all` and `=list=`).
+  if ((MF->hasBBLabels() || MBB.isBeginSection()) && !MBB.isEntryBlock())
+    return true;
+  // A label is needed for any block with at least one predecessor (when that
+  // predecessor is not the fallthrough predecessor, or if it is an EH funclet
+  // entry, or if a label is forced).
+  return !MBB.pred_empty() &&
+         (!isBlockOnlyReachableByFallthrough(&MBB) || MBB.isEHFuncletEntry() ||
+          MBB.hasLabelMustBeEmitted());
+}
+
 /// isBlockOnlyReachableByFallthough - Return true if the basic block has
 /// exactly one predecessor and the control transfer mechanism between
 /// the predecessor and this block is a fall-through.
 bool AsmPrinter::
 isBlockOnlyReachableByFallthrough(const MachineBasicBlock *MBB) const {
-  // With BasicBlock Sections, beginning of the section is not a fallthrough.
-  if (MBB->isBeginSection())
-    return false;
-
   // If this is a landing pad, it isn't a fall through.  If it has no preds,
   // then nothing falls through to it.
   if (MBB->isEHPad() || MBB->pred_empty())

llvm/lib/CodeGen/MachineBasicBlock.cpp

@@ -266,6 +266,10 @@ bool MachineBasicBlock::hasEHPadSuccessor() const {
   return false;
 }
 
+bool MachineBasicBlock::isEntryBlock() const {
+  return getParent()->begin() == getIterator();
+}
+
 #if !defined(NDEBUG) || defined(LLVM_ENABLE_DUMP)
 LLVM_DUMP_METHOD void MachineBasicBlock::dump() const {
   print(dbgs());

llvm/test/CodeGen/X86/basic-block-sections-unreachable.ll

@@ -0,0 +1,18 @@
+; Check that basic block section is emitted when a non-entry block has no predecessors.
+; RUN: llc < %s -mtriple=x86_64 -O0 -basic-block-sections=all | FileCheck %s --check-prefix=CHECK-SECTIONS
+; RUN: llc < %s -mtriple=x86_64 -O0 | FileCheck %s --check-prefix=CHECK-NOSECTIONS
+define void @foo(i32* %bar) {
+  %v = load i32, i32* %bar
+  switch i32 %v, label %default [
+    i32 0, label %target
+  ]
+target:
+  ret void
+;; This is the block which will not have any predecessors. If the block is not garbage collected, it must
+;; be placed in a basic block section with a corresponding symbol.
+default:
+  unreachable
+; CHECK-NOSECTIONS:     # %bb.2:     # %default
+; CHECK-SECTIONS:       .section .text,"ax",@progbits,unique,2
+; CHECK-SECTIONS-NEXT:  foo.2:       # %default
+}