[BasicBlockSections] Using MBBSectionID as DenseMap key #97295
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
getSectionIDNum may return same value for two different MBBSectionID. e.g. A Cold type MBBSectionID with number 0 and a Default type MBBSectionID with number 2 get same value 2 from getSectionIDNum. This may lead to overwrite of MBBSectionRanges. Using MBBSectionID itself as DenseMap key is better choice.
|
@llvm/pr-subscribers-debuginfo Author: Haohai Wen (HaohaiWen) ChangesgetSectionIDNum may return same value for two different MBBSectionID. Full diff: https://github.com/llvm/llvm-project/pull/97295.diff 6 Files Affected:
diff --git a/llvm/include/llvm/CodeGen/AsmPrinter.h b/llvm/include/llvm/CodeGen/AsmPrinter.h
index 011f8c6534b6a..919454b819e36 100644
--- a/llvm/include/llvm/CodeGen/AsmPrinter.h
+++ b/llvm/include/llvm/CodeGen/AsmPrinter.h
@@ -136,7 +136,7 @@ class AsmPrinter : public MachineFunctionPass {
MCSymbol *BeginLabel, *EndLabel;
};
- MapVector<unsigned, MBBSectionRange> MBBSectionRanges;
+ MapVector<MBBSectionID, MBBSectionRange> MBBSectionRanges;
/// Map global GOT equivalent MCSymbols to GlobalVariables and keep track of
/// its number of uses by other globals.
@@ -173,7 +173,7 @@ class AsmPrinter : public MachineFunctionPass {
/// Map a basic block section ID to the exception symbol associated with that
/// section. Map entries are assigned and looked up via
/// AsmPrinter::getMBBExceptionSym.
- DenseMap<unsigned, MCSymbol *> MBBSectionExceptionSyms;
+ DenseMap<MBBSectionID, MCSymbol *> MBBSectionExceptionSyms;
// The symbol used to represent the start of the current BB section of the
// function. This is used to calculate the size of the BB section.
diff --git a/llvm/include/llvm/CodeGen/MachineBasicBlock.h b/llvm/include/llvm/CodeGen/MachineBasicBlock.h
index e4919ecabd705..562d37ef32f54 100644
--- a/llvm/include/llvm/CodeGen/MachineBasicBlock.h
+++ b/llvm/include/llvm/CodeGen/MachineBasicBlock.h
@@ -13,6 +13,7 @@
#ifndef LLVM_CODEGEN_MACHINEBASICBLOCK_H
#define LLVM_CODEGEN_MACHINEBASICBLOCK_H
+#include "llvm/ADT/DenseMapInfo.h"
#include "llvm/ADT/GraphTraits.h"
#include "llvm/ADT/SparseBitVector.h"
#include "llvm/ADT/ilist.h"
@@ -74,6 +75,25 @@ struct MBBSectionID {
MBBSectionID(SectionType T) : Type(T), Number(0) {}
};
+template <> struct DenseMapInfo<MBBSectionID> {
+ using TypeInfo = DenseMapInfo<MBBSectionID::SectionType>;
+ using NumberInfo = DenseMapInfo<unsigned>;
+
+ static inline MBBSectionID getEmptyKey() {
+ return MBBSectionID(NumberInfo::getEmptyKey());
+ }
+ static inline MBBSectionID getTombstoneKey() {
+ return MBBSectionID(NumberInfo::getTombstoneKey());
+ }
+ static unsigned getHashValue(const MBBSectionID &SecID) {
+ return detail::combineHashValue(TypeInfo::getHashValue(SecID.Type),
+ NumberInfo::getHashValue(SecID.Number));
+ }
+ static bool isEqual(const MBBSectionID &LHS, const MBBSectionID &RHS) {
+ return LHS == RHS;
+ }
+};
+
// This structure represents the information for a basic block pertaining to
// the basic block sections profile.
struct UniqueBBID {
@@ -658,12 +678,6 @@ class MachineBasicBlock
/// Returns the section ID of this basic block.
MBBSectionID getSectionID() const { return SectionID; }
- /// Returns the unique section ID number of this basic block.
- unsigned getSectionIDNum() const {
- return ((unsigned)MBBSectionID::SectionType::Cold) -
- ((unsigned)SectionID.Type) + SectionID.Number;
- }
-
/// Sets the fixed BBID of this basic block.
void setBBID(const UniqueBBID &V) {
assert(!BBID.has_value() && "Cannot change BBID.");
diff --git a/llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp b/llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp
index 6d6ceed053fd0..b7eae5cc82936 100644
--- a/llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp
+++ b/llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp
@@ -1410,7 +1410,7 @@ void AsmPrinter::emitBBAddrMapSection(const MachineFunction &MF) {
OutStreamer->emitULEB128IntValue(MBBSectionRanges.size());
}
// Number of blocks in each MBB section.
- MapVector<unsigned, unsigned> MBBSectionNumBlocks;
+ MapVector<MBBSectionID, unsigned> MBBSectionNumBlocks;
const MCSymbol *PrevMBBEndSymbol = nullptr;
if (!Features.MultiBBRange) {
OutStreamer->AddComment("function address");
@@ -1424,7 +1424,7 @@ void AsmPrinter::emitBBAddrMapSection(const MachineFunction &MF) {
BBCount++;
if (MBB.isEndSection()) {
// Store each section's basic block count when it ends.
- MBBSectionNumBlocks[MBB.getSectionIDNum()] = BBCount;
+ MBBSectionNumBlocks[MBB.getSectionID()] = BBCount;
// Reset the count for the next section.
BBCount = 0;
}
@@ -1440,8 +1440,7 @@ void AsmPrinter::emitBBAddrMapSection(const MachineFunction &MF) {
OutStreamer->AddComment("base address");
OutStreamer->emitSymbolValue(MBBSymbol, getPointerSize());
OutStreamer->AddComment("number of basic blocks");
- OutStreamer->emitULEB128IntValue(
- MBBSectionNumBlocks[MBB.getSectionIDNum()]);
+ OutStreamer->emitULEB128IntValue(MBBSectionNumBlocks[MBB.getSectionID()]);
PrevMBBEndSymbol = MBBSymbol;
}
// TODO: Remove this check when version 1 is deprecated.
@@ -1897,7 +1896,9 @@ void AsmPrinter::emitFunctionBody() {
OutContext);
OutStreamer->emitELFSize(CurrentSectionBeginSym, SizeExp);
}
- MBBSectionRanges[MBB.getSectionIDNum()] =
+ assert(!MBBSectionRanges.contains(MBB.getSectionID()) &&
+ "Overwrite section range");
+ MBBSectionRanges[MBB.getSectionID()] =
MBBSectionRange{CurrentSectionBeginSym, MBB.getEndSymbol()};
}
}
@@ -2018,7 +2019,9 @@ void AsmPrinter::emitFunctionBody() {
HI.Handler->markFunctionEnd();
}
- MBBSectionRanges[MF->front().getSectionIDNum()] =
+ assert(!MBBSectionRanges.contains(MF->front().getSectionID()) &&
+ "Overwrite section range");
+ MBBSectionRanges[MF->front().getSectionID()] =
MBBSectionRange{CurrentFnBegin, CurrentFnEnd};
// Print out jump tables referenced by the function.
@@ -2582,7 +2585,7 @@ bool AsmPrinter::doFinalization(Module &M) {
}
MCSymbol *AsmPrinter::getMBBExceptionSym(const MachineBasicBlock &MBB) {
- auto Res = MBBSectionExceptionSyms.try_emplace(MBB.getSectionIDNum());
+ auto Res = MBBSectionExceptionSyms.try_emplace(MBB.getSectionID());
if (Res.second)
Res.first->second = createTempSymbol("exception");
return Res.first->second;
diff --git a/llvm/lib/CodeGen/AsmPrinter/DwarfCompileUnit.cpp b/llvm/lib/CodeGen/AsmPrinter/DwarfCompileUnit.cpp
index c1e7f01f0eba5..c1e8355353cfd 100644
--- a/llvm/lib/CodeGen/AsmPrinter/DwarfCompileUnit.cpp
+++ b/llvm/lib/CodeGen/AsmPrinter/DwarfCompileUnit.cpp
@@ -684,7 +684,7 @@ void DwarfCompileUnit::attachRangesOrLowHighPC(
// the order of blocks will be frozen beyond this point.
do {
if (MBB->sameSection(EndMBB) || MBB->isEndSection()) {
- auto MBBSectionRange = Asm->MBBSectionRanges[MBB->getSectionIDNum()];
+ auto MBBSectionRange = Asm->MBBSectionRanges[MBB->getSectionID()];
List.push_back(
{MBB->sameSection(BeginMBB) ? BeginLabel
: MBBSectionRange.BeginLabel,
diff --git a/llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp b/llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp
index 2addf938c8b63..80cd5ec501f25 100644
--- a/llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp
+++ b/llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp
@@ -1713,7 +1713,7 @@ bool DwarfDebug::buildLocationList(SmallVectorImpl<DebugLocEntry> &DebugLoc,
const MCSymbol *EndLabel;
if (std::next(EI) == Entries.end()) {
const MachineBasicBlock &EndMBB = Asm->MF->back();
- EndLabel = Asm->MBBSectionRanges[EndMBB.getSectionIDNum()].EndLabel;
+ EndLabel = Asm->MBBSectionRanges[EndMBB.getSectionID()].EndLabel;
if (EI->isClobber())
EndMI = EI->getInstr();
}
@@ -2064,7 +2064,7 @@ void DwarfDebug::beginInstruction(const MachineInstr *MI) {
bool PrevInstInSameSection =
(!PrevInstBB ||
- PrevInstBB->getSectionIDNum() == MI->getParent()->getSectionIDNum());
+ PrevInstBB->getSectionID() == MI->getParent()->getSectionID());
if (DL == PrevInstLoc && PrevInstInSameSection) {
// If we have an ongoing unspecified location, nothing to do here.
if (!DL)
diff --git a/llvm/lib/CodeGen/AsmPrinter/EHStreamer.cpp b/llvm/lib/CodeGen/AsmPrinter/EHStreamer.cpp
index 32239535e4d02..1c603f5988ad1 100644
--- a/llvm/lib/CodeGen/AsmPrinter/EHStreamer.cpp
+++ b/llvm/lib/CodeGen/AsmPrinter/EHStreamer.cpp
@@ -253,8 +253,8 @@ void EHStreamer::computeCallSiteTable(
// We start a call-site range upon function entry and at the beginning of
// every basic block section.
CallSiteRanges.push_back(
- {Asm->MBBSectionRanges[MBB.getSectionIDNum()].BeginLabel,
- Asm->MBBSectionRanges[MBB.getSectionIDNum()].EndLabel,
+ {Asm->MBBSectionRanges[MBB.getSectionID()].BeginLabel,
+ Asm->MBBSectionRanges[MBB.getSectionID()].EndLabel,
Asm->getMBBExceptionSym(MBB), CallSites.size()});
PreviousIsInvoke = false;
SawPotentiallyThrowing = false;
|
williamweixiao
approved these changes
Jul 2, 2024
|
I'd like to merge it if no any objection. |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/72/builds/777 Here is the relevant piece of the build log for the reference: |
kbluck
pushed a commit
to kbluck/llvm-project
that referenced
this pull request
Jul 6, 2024
getSectionIDNum may return same value for two different MBBSectionID. e.g. A Cold type MBBSectionID with number 0 and a Default type MBBSectionID with number 2 get same value 2 from getSectionIDNum. This may lead to overwrite of MBBSectionRanges. Using MBBSectionID itself as DenseMap key is better choice.
|
Sorry, I was OOO and didn't get a chance to look at this. You mention that |
Oh sorry. It's typo... |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
getSectionIDNum may return same value for two different MBBSectionID.
e.g. A Cold type MBBSectionID with number 2 and a Default type
MBBSectionID with number 0 get same value 2 from getSectionIDNum. This
may lead to overwrite of MBBSectionRanges. Using MBBSectionID itself
as DenseMap key is better choice.