Skip to content
Merged
Show file tree
Hide file tree
Changes from 15 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 18 additions & 8 deletions llvm/include/llvm/Object/ELFTypes.h
Original file line number Diff line number Diff line change
Expand Up @@ -834,6 +834,7 @@ struct BBAddrMap {
bool OmitBBEntries : 1;
bool CallsiteEndOffsets : 1;
bool BBHash : 1;
bool PostLinkCfg : 1;

bool hasPGOAnalysis() const { return FuncEntryCount || BBFreq || BrProb; }

Expand All @@ -847,7 +848,8 @@ struct BBAddrMap {
(static_cast<uint8_t>(MultiBBRange) << 3) |
(static_cast<uint8_t>(OmitBBEntries) << 4) |
(static_cast<uint8_t>(CallsiteEndOffsets) << 5) |
(static_cast<uint8_t>(BBHash) << 6);
(static_cast<uint8_t>(BBHash) << 6) |
(static_cast<uint8_t>(PostLinkCfg) << 7);
}

// Decodes from minimum bit width representation and validates no
Expand All @@ -857,7 +859,7 @@ struct BBAddrMap {
static_cast<bool>(Val & (1 << 0)), static_cast<bool>(Val & (1 << 1)),
static_cast<bool>(Val & (1 << 2)), static_cast<bool>(Val & (1 << 3)),
static_cast<bool>(Val & (1 << 4)), static_cast<bool>(Val & (1 << 5)),
static_cast<bool>(Val & (1 << 6))};
static_cast<bool>(Val & (1 << 6)), static_cast<bool>(Val & (1 << 7))};
if (Feat.encode() != Val)
return createStringError(
std::error_code(), "invalid encoding for BBAddrMap::Features: 0x%x",
Expand All @@ -867,10 +869,11 @@ struct BBAddrMap {

bool operator==(const Features &Other) const {
return std::tie(FuncEntryCount, BBFreq, BrProb, MultiBBRange,
OmitBBEntries, CallsiteEndOffsets, BBHash) ==
OmitBBEntries, CallsiteEndOffsets, BBHash, PostLinkCfg) ==
std::tie(Other.FuncEntryCount, Other.BBFreq, Other.BrProb,
Other.MultiBBRange, Other.OmitBBEntries,
Other.CallsiteEndOffsets, Other.BBHash);
Other.CallsiteEndOffsets, Other.BBHash,
Other.PostLinkCfg);
}
};

Expand Down Expand Up @@ -1010,23 +1013,30 @@ struct PGOAnalysisMap {
/// probability associated with it.
struct SuccessorEntry {
/// Unique ID of this successor basic block.
uint32_t ID;
uint32_t ID = 0;
/// Branch Probability of the edge to this successor taken from MBPI.
BranchProbability Prob;
/// Raw edge count from the post link profile (e.g., from bolt or
/// propeller).
uint64_t PostLinkFreq = 0;

bool operator==(const SuccessorEntry &Other) const {
return std::tie(ID, Prob) == std::tie(Other.ID, Other.Prob);
return std::tie(ID, Prob, PostLinkFreq) ==
std::tie(Other.ID, Other.Prob, Other.PostLinkFreq);
}
};

/// Block frequency taken from MBFI
BlockFrequency BlockFreq;
/// Raw block count taken from the post link profile (e.g., from bolt or
/// propeller).
uint64_t PostLinkBlockFreq = 0;
/// List of successors of the current block
llvm::SmallVector<SuccessorEntry, 2> Successors;

bool operator==(const PGOBBEntry &Other) const {
return std::tie(BlockFreq, Successors) ==
std::tie(Other.BlockFreq, Other.Successors);
return std::tie(BlockFreq, PostLinkBlockFreq, Successors) ==
std::tie(Other.BlockFreq, PostLinkBlockFreq, Other.Successors);
}
};

Expand Down
2 changes: 2 additions & 0 deletions llvm/include/llvm/ObjectYAML/ELFYAML.h
Original file line number Diff line number Diff line change
Expand Up @@ -203,8 +203,10 @@ struct PGOAnalysisMapEntry {
struct SuccessorEntry {
uint32_t ID;
llvm::yaml::Hex32 BrProb;
std::optional<uint32_t> PostLinkBrFreq;
};
std::optional<uint64_t> BBFreq;
std::optional<uint32_t> PostLinkBBFreq;
std::optional<std::vector<SuccessorEntry>> Successors;
};
std::optional<uint64_t> FuncEntryCount;
Expand Down
2 changes: 1 addition & 1 deletion llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1443,7 +1443,7 @@ getBBAddrMapFeature(const MachineFunction &MF, int NumMBBSectionRanges,
MF.hasBBSections() && NumMBBSectionRanges > 1,
// Use static_cast to avoid breakage of tests on windows.
static_cast<bool>(BBAddrMapSkipEmitBBEntries), HasCalls,
static_cast<bool>(EmitBBHash)};
static_cast<bool>(EmitBBHash), false};
}

void AsmPrinter::emitBBAddrMapSection(const MachineFunction &MF) {
Expand Down
22 changes: 19 additions & 3 deletions llvm/lib/Object/ELF.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -838,7 +838,7 @@ decodeBBAddrMapImpl(const ELFFile<ELFT> &EF,
Version = Data.getU8(Cur);
if (!Cur)
break;
if (Version < 2 || Version > 4)
if (Version < 2 || Version > 5)
return createError("unsupported SHT_LLVM_BB_ADDR_MAP version: " +
Twine(static_cast<int>(Version)));
Feature = Data.getU8(Cur); // Feature byte
Expand All @@ -858,6 +858,11 @@ decodeBBAddrMapImpl(const ELFFile<ELFT> &EF,
"basic block hash feature is enabled: version = " +
Twine(static_cast<int>(Version)) +
" feature = " + Twine(static_cast<int>(Feature)));
if (FeatEnable.PostLinkCfg && Version < 5)
return createError("version should be >= 5 for SHT_LLVM_BB_ADDR_MAP when "
"basic block hash feature is enabled: version = " +
Twine(static_cast<int>(Version)) +
" feature = " + Twine(static_cast<int>(Feature)));
uint32_t NumBlocksInBBRange = 0;
uint32_t NumBBRanges = 1;
typename ELFFile<ELFT>::uintX_t RangeBaseAddress = 0;
Expand Down Expand Up @@ -946,6 +951,10 @@ decodeBBAddrMapImpl(const ELFFile<ELFT> &EF,
uint64_t BBF = FeatEnable.BBFreq
? readULEB128As<uint64_t>(Data, Cur, ULEBSizeErr)
: 0;
uint32_t PostLinkBBFreq =
FeatEnable.PostLinkCfg
? readULEB128As<uint32_t>(Data, Cur, ULEBSizeErr)
: 0;

// Branch probability
llvm::SmallVector<PGOAnalysisMap::PGOBBEntry::SuccessorEntry, 2>
Expand All @@ -955,13 +964,20 @@ decodeBBAddrMapImpl(const ELFFile<ELFT> &EF,
for (uint64_t I = 0; I < SuccCount; ++I) {
uint32_t BBID = readULEB128As<uint32_t>(Data, Cur, ULEBSizeErr);
uint32_t BrProb = readULEB128As<uint32_t>(Data, Cur, ULEBSizeErr);
uint32_t PostLinkFreq =
FeatEnable.PostLinkCfg
? readULEB128As<uint32_t>(Data, Cur, ULEBSizeErr)
: 0;

if (PGOAnalyses)
Successors.push_back({BBID, BranchProbability::getRaw(BrProb)});
Successors.push_back(
{BBID, BranchProbability::getRaw(BrProb), PostLinkFreq});
}
}

if (PGOAnalyses)
PGOBBEntries.push_back({BlockFrequency(BBF), std::move(Successors)});
PGOBBEntries.push_back(
{BlockFrequency(BBF), PostLinkBBFreq, std::move(Successors)});
}

if (PGOAnalyses)
Expand Down
8 changes: 6 additions & 2 deletions llvm/lib/ObjectYAML/ELFEmitter.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1465,7 +1465,7 @@ void ELFState<ELFT>::writeSectionContent(
for (const auto &[Idx, E] : llvm::enumerate(*Section.Entries)) {
// Write version and feature values.
if (Section.Type == llvm::ELF::SHT_LLVM_BB_ADDR_MAP) {
if (E.Version > 4)
if (E.Version > 5)
WithColor::warning() << "unsupported SHT_LLVM_BB_ADDR_MAP version: "
<< static_cast<int>(E.Version)
<< "; encoding using the most recent version";
Expand Down Expand Up @@ -1556,11 +1556,15 @@ void ELFState<ELFT>::writeSectionContent(
for (const auto &PGOBBE : PGOBBEntries) {
if (PGOBBE.BBFreq)
SHeader.sh_size += CBA.writeULEB128(*PGOBBE.BBFreq);
if (FeatureOrErr->PostLinkCfg || PGOBBE.PostLinkBBFreq.has_value())
SHeader.sh_size += CBA.writeULEB128(PGOBBE.PostLinkBBFreq.value_or(0));
if (PGOBBE.Successors) {
SHeader.sh_size += CBA.writeULEB128(PGOBBE.Successors->size());
for (const auto &[ID, BrProb] : *PGOBBE.Successors) {
for (const auto &[ID, BrProb, PostLinkBrFreq] : *PGOBBE.Successors) {
SHeader.sh_size += CBA.writeULEB128(ID);
SHeader.sh_size += CBA.writeULEB128(BrProb);
if (FeatureOrErr->PostLinkCfg || PostLinkBrFreq.has_value())
SHeader.sh_size += CBA.writeULEB128(PostLinkBrFreq.value_or(0));
}
}
}
Expand Down
2 changes: 2 additions & 0 deletions llvm/lib/ObjectYAML/ELFYAML.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1920,6 +1920,7 @@ void MappingTraits<ELFYAML::PGOAnalysisMapEntry::PGOBBEntry>::mapping(
IO &IO, ELFYAML::PGOAnalysisMapEntry::PGOBBEntry &E) {
assert(IO.getContext() && "The IO context is not initialized");
IO.mapOptional("BBFreq", E.BBFreq);
IO.mapOptional("PostLinkBBFreq", E.PostLinkBBFreq);
IO.mapOptional("Successors", E.Successors);
}

Expand All @@ -1929,6 +1930,7 @@ void MappingTraits<ELFYAML::PGOAnalysisMapEntry::PGOBBEntry::SuccessorEntry>::
assert(IO.getContext() && "The IO context is not initialized");
IO.mapRequired("ID", E.ID);
IO.mapRequired("BrProb", E.BrProb);
IO.mapOptional("PostLinkBrFreq", E.PostLinkBrFreq);
}

void MappingTraits<ELFYAML::GnuHashHeader>::mapping(IO &IO,
Expand Down
17 changes: 11 additions & 6 deletions llvm/test/tools/llvm-readobj/ELF/bb-addr-map-pgo-analysis-map.test
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@

## Check that a malformed section can be handled.
# RUN: yaml2obj %s -DBITS=32 -DSIZE=24 -o %t2.o
# RUN: llvm-readobj %t2.o --bb-addr-map 2>&1 | FileCheck --match-full-lines %s -DOFFSET=0x00000018 -DFILE=%t2.o --check-prefix=TRUNCATED
# RUN: llvm-readobj %t2.o --bb-addr-map 2>&1 | FileCheck --match-full-lines %s -DOFFSET=0x00000014 -DFILE=%t2.o --check-prefix=TRUNCATED

## Check that missing features can be handled.
# RUN: yaml2obj %s -DBITS=32 -DFEATURE=0x2 -o %t3.o
Expand Down Expand Up @@ -59,17 +59,20 @@
# CHECK-NEXT: {
# RAW-NEXT: Frequency: 100
# PRETTY-NEXT: Frequency: 1.0
# CHECK-NEXT: PostLink Frequency: 10
# CHECK-NEXT: Successors [
# CHECK-NEXT: {
# CHECK-NEXT: ID: 2
# RAW-NEXT: Probability: 0x80000000
# PRETTY-NEXT: Probability: 0x80000000 / 0x80000000 = 100.00%
# CHECK-NEXT: PostLink Probability: 7
# CHECK-NEXT: }
# CHECK-NEXT: ]
# CHECK-NEXT: }
# CHECK-NEXT: {
# RAW-NEXT: Frequency: 100
# PRETTY-NEXT: Frequency: 1.0
# CHECK-NEXT: PostLink Frequency: 0
# CHECK-NEXT: Successors [
# CHECK-NEXT: ]
# CHECK-NEXT: }
Expand Down Expand Up @@ -172,8 +175,8 @@ Sections:
ShSize: [[SIZE=<none>]]
Link: .text
Entries:
- Version: 2
Feature: 0x7
- Version: 5
Feature: 0x87
BBRanges:
- BaseAddress: [[ADDR=0x11111]]
BBEntries:
Expand All @@ -197,10 +200,12 @@ Sections:
PGOAnalyses:
- FuncEntryCount: 100
PGOBBEntries:
- BBFreq: 100
- BBFreq: 100
PostLinkBBFreq: 10
Successors:
- ID: 2
BrProb: 0x80000000
- ID: 2
BrProb: 0x80000000
PostLinkBrFreq: 7
- BBFreq: 100
Successors: []
- FuncEntryCount: 8888
Expand Down
49 changes: 29 additions & 20 deletions llvm/test/tools/obj2yaml/ELF/bb-addr-map-pgo-analysis-map.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
# VALID-NEXT: Type: SHT_LLVM_BB_ADDR_MAP
# VALID-NEXT: Entries:
# VALID-NEXT: - Version: 2
# VALID-NEXT: Feature: 0x7
# VALID-NEXT: Feature: 0x87
## The 'BaseAddress' field is omitted when it's zero.
# VALID-NEXT: BBRanges:
# VALID-NEXT: - BBEntries:
Expand Down Expand Up @@ -43,17 +43,23 @@
# VALID-NEXT: PGOAnalyses:
# VALID-NEXT: - FuncEntryCount: 100
# VALID-NEXT: PGOBBEntries:
# VALID-NEXT: - BBFreq: 100
# VALID-NEXT: - BBFreq: 100
# VALID-NEXT: PostLinkBBFreq: 10
# VALID-NEXT: Successors:
# VALID-NEXT: - ID: 2
# VALID-NEXT: BrProb: 0x80000000
# VALID-NEXT: - ID: 4
# VALID-NEXT: BrProb: 0x80000000
# VALID-NEXT: - BBFreq: 50
# VALID-NEXT: - ID: 2
# VALID-NEXT: BrProb: 0x80000000
# VALID-NEXT: PostLinkBrFreq: 7
# VALID-NEXT: - ID: 4
# VALID-NEXT: BrProb: 0x80000000
# VALID-NEXT: PostLinkBrFreq: 0
# VALID-NEXT: - BBFreq: 50
# VALID-NEXT: PostLinkBBFreq: 0
# VALID-NEXT: Successors:
# VALID-NEXT: - ID: 4
# VALID-NEXT: BrProb: 0xFFFFFFFF
# VALID-NEXT: - BBFreq: 100
# VALID-NEXT: - ID: 4
# VALID-NEXT: BrProb: 0xFFFFFFFF
# VALID-NEXT: PostLinkBrFreq: 0
# VALID-NEXT: - BBFreq: 100
# VALID-NEXT: PostLinkBBFreq: 3
# VALID-NEXT: Successors: []
# VALID-NEXT: PGOBBEntries:
# VALID-NEXT: - BBFreq: 20
Expand All @@ -69,7 +75,7 @@ Sections:
ShSize: [[SIZE=<none>]]
Entries:
- Version: 2
Feature: 0x7
Feature: 0x87
BBRanges:
- BaseAddress: 0x0
BBEntries:
Expand Down Expand Up @@ -97,17 +103,20 @@ Sections:
PGOAnalyses:
- FuncEntryCount: 100
PGOBBEntries:
- BBFreq: 100
- BBFreq: 100
PostLinkBBFreq: 10
Successors:
- ID: 2
BrProb: 0x80000000
- ID: 4
BrProb: 0x80000000
- BBFreq: 50
- ID: 2
BrProb: 0x80000000
PostLinkBrFreq: 7
- ID: 4
BrProb: 0x80000000
- BBFreq: 50
Successors:
- ID: 4
BrProb: 0xFFFFFFFF
- BBFreq: 100
- ID: 4
BrProb: 0xFFFFFFFF
- BBFreq: 100
PostLinkBBFreq: 3
Successors: []
- PGOBBEntries:
- BBFreq: 20
Expand Down
25 changes: 14 additions & 11 deletions llvm/test/tools/yaml2obj/ELF/bb-addr-map-pgo-analysis-map.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,9 @@
# Case 4: Specify Entries.
# CHECK: Name: .llvm_bb_addr_map (1)
# CHECK: SectionData (
# CHECK-NEXT: 0000: 02072000 00000000 0000010B 010203E8
# CHECK-NEXT: 0010: 07E80702 0CEEDDBB F70E0D91 A2C48801
# CHECK-NEXT: 0000: 02872000 00000000 0000010B 010203E8
# CHECK-NEXT: 0010: 07E80764 020CEEDD BBF70E28 0D91A2C4
# CHECK-NEXT: 0020: 880100
# CHECK-NEXT: )

# Case 7: Not including a field which is enabled in feature doesn't emit value
Expand All @@ -31,7 +32,7 @@ Sections:
Type: SHT_LLVM_BB_ADDR_MAP
Entries:
- Version: 2
Feature: 0x7
Feature: 0x87
BBRanges:
- BaseAddress: 0x0000000000000020
BBEntries:
Expand All @@ -42,12 +43,14 @@ Sections:
PGOAnalyses:
- FuncEntryCount: 1000
PGOBBEntries:
- BBFreq: 1000
- BBFreq: 1000
PostLinkBBFreq: 100
Successors:
- ID: 12
BrProb: 0xeeeeeeee
- ID: 13
BrProb: 0x11111111
- ID: 12
BrProb: 0xeeeeeeee
PostLinkBrFreq: 40
- ID: 13
BrProb: 0x11111111

## 2) According to feature we have FuncEntryCount but none is provided in yaml
- Name: '.llvm_bb_addr_map (2)'
Expand All @@ -65,8 +68,8 @@ Sections:
Metadata: 0x00000003

## Check that yaml2obj generates a warning when we use unsupported feature.
# RUN: yaml2obj --docnum=2 %s 2>&1 | FileCheck %s --check-prefix=INVALID-FEATURE
# INVALID-FEATURE: warning: invalid encoding for BBAddrMap::Features: 0xf0
# RUN: not yaml2obj --docnum=2 %s 2>&1 | FileCheck %s --check-prefix=INVALID-FEATURE
# INVALID-FEATURE: error: out of range hex8 number
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Comment says warning but test is checking for an error. Perhaps we should just up the type from hex8 to a larger value now, to preserve the existing more useful warning? I've no objection to keeping it as hex8 though, if you just want to update the comment.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point. I did briefly think about bumping it to hex16, but though I would do it a later PR. But now realized that would also require a version bump. So why not doing it here since we know the next upcoming feature will need the additional byte.

Also added a test to verify warnings related to old versions not supporting newer features.


--- !ELF
FileHeader:
Expand All @@ -79,4 +82,4 @@ Sections:
Entries:
- Version: 2
## Specify unsupported feature
Feature: 0xF0
Feature: 0x100
Loading