[llvm] 0b2f4cd - Explicitly check for entry basic block, rather than relying on MachineBasicBlock::pred_empty.
Rahman Lavaee via llvm-commits
llvm-commits at lists.llvm.org
Mon Oct 26 16:16:28 PDT 2020
Author: Rahman Lavaee
Date: 2020-10-26T16:15:56-07:00
New Revision: 0b2f4cdf2bd4e48974099cd441afaabc18ead227
URL: https://github.com/llvm/llvm-project/commit/0b2f4cdf2bd4e48974099cd441afaabc18ead227
DIFF: https://github.com/llvm/llvm-project/commit/0b2f4cdf2bd4e48974099cd441afaabc18ead227.diff
LOG: 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
Added:
llvm/test/CodeGen/X86/basic-block-sections-unreachable.ll
Modified:
llvm/include/llvm/CodeGen/AsmPrinter.h
llvm/include/llvm/CodeGen/MachineBasicBlock.h
llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp
llvm/lib/CodeGen/MachineBasicBlock.cpp
Removed:
################################################################################
diff --git a/llvm/include/llvm/CodeGen/AsmPrinter.h b/llvm/include/llvm/CodeGen/AsmPrinter.h
index 3056568ccf98..56a1c609acfb 100644
--- a/llvm/include/llvm/CodeGen/AsmPrinter.h
+++ b/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
diff --git a/llvm/include/llvm/CodeGen/MachineBasicBlock.h b/llvm/include/llvm/CodeGen/MachineBasicBlock.h
index f4e36ab8d645..2bad64c6cc2e 100644
--- a/llvm/include/llvm/CodeGen/MachineBasicBlock.h
+++ b/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; }
diff --git a/llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp b/llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp
index f491fb137427..0a4d7b13ecd3 100644
--- a/llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp
+++ b/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())
diff --git a/llvm/lib/CodeGen/MachineBasicBlock.cpp b/llvm/lib/CodeGen/MachineBasicBlock.cpp
index 13ece5ed7d1c..c203ad1e1108 100644
--- a/llvm/lib/CodeGen/MachineBasicBlock.cpp
+++ b/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());
diff --git a/llvm/test/CodeGen/X86/basic-block-sections-unreachable.ll b/llvm/test/CodeGen/X86/basic-block-sections-unreachable.ll
new file mode 100644
index 000000000000..6501d7ba44ec
--- /dev/null
+++ b/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", at progbits,unique,2
+; CHECK-SECTIONS-NEXT: foo.2: # %default
+}
More information about the llvm-commits
mailing list