[SHT_LLVM_BB_ADDR] Implement ELF and YAML support for Propeller CFG data in PGO analysis map. #164914
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
|
@llvm/pr-subscribers-llvm-binary-utilities Author: Rahman Lavaee (rlavaee) ChangesThis PR implements the ELF support for Propeller CFG in PGO analysis map as discussed in RFC. Patch is 20.40 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/164914.diff 12 Files Affected:
diff --git a/llvm/include/llvm/Object/ELFTypes.h b/llvm/include/llvm/Object/ELFTypes.h
index e9a417d3d4fb3..101ead91df4bd 100644
--- a/llvm/include/llvm/Object/ELFTypes.h
+++ b/llvm/include/llvm/Object/ELFTypes.h
@@ -834,6 +834,7 @@ struct BBAddrMap {
bool OmitBBEntries : 1;
bool CallsiteEndOffsets : 1;
bool BBHash : 1;
+ bool PropellerCfg : 1;
bool hasPGOAnalysis() const { return FuncEntryCount || BBFreq || BrProb; }
@@ -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>(PropellerCfg) << 7);
}
// Decodes from minimum bit width representation and validates no
@@ -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",
@@ -867,10 +869,12 @@ struct BBAddrMap {
bool operator==(const Features &Other) const {
return std::tie(FuncEntryCount, BBFreq, BrProb, MultiBBRange,
- OmitBBEntries, CallsiteEndOffsets, BBHash) ==
+ OmitBBEntries, CallsiteEndOffsets, BBHash,
+ PropellerCfg) ==
std::tie(Other.FuncEntryCount, Other.BBFreq, Other.BrProb,
Other.MultiBBRange, Other.OmitBBEntries,
- Other.CallsiteEndOffsets, Other.BBHash);
+ Other.CallsiteEndOffsets, Other.BBHash,
+ Other.PropellerCfg);
}
};
@@ -1013,20 +1017,25 @@ struct PGOAnalysisMap {
uint32_t ID;
/// Branch Probability of the edge to this successor taken from MBPI.
BranchProbability Prob;
+ /// Edge frequency from Propeller.
+ uint32_t PropellerFreq;
bool operator==(const SuccessorEntry &Other) const {
- return std::tie(ID, Prob) == std::tie(Other.ID, Other.Prob);
+ return std::tie(ID, Prob, PropellerFreq) ==
+ std::tie(Other.ID, Other.Prob, Other.PropellerFreq);
}
};
/// Block frequency taken from MBFI
BlockFrequency BlockFreq;
+ /// Block frequency taken from Propeller.
+ uint32_t PropellerBlockFreq;
/// 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, PropellerBlockFreq, Successors) ==
+ std::tie(Other.BlockFreq, PropellerBlockFreq, Other.Successors);
}
};
diff --git a/llvm/include/llvm/ObjectYAML/ELFYAML.h b/llvm/include/llvm/ObjectYAML/ELFYAML.h
index a7c7c7c436dc2..1b7a4018369c3 100644
--- a/llvm/include/llvm/ObjectYAML/ELFYAML.h
+++ b/llvm/include/llvm/ObjectYAML/ELFYAML.h
@@ -203,8 +203,10 @@ struct PGOAnalysisMapEntry {
struct SuccessorEntry {
uint32_t ID;
llvm::yaml::Hex32 BrProb;
+ std::optional<uint32_t> PropellerBrFreq;
};
std::optional<uint64_t> BBFreq;
+ std::optional<uint32_t> PropellerBBFreq;
std::optional<std::vector<SuccessorEntry>> Successors;
};
std::optional<uint64_t> FuncEntryCount;
diff --git a/llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp b/llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp
index 8aa488f0efd8f..8f000dbba327a 100644
--- a/llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp
+++ b/llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp
@@ -1444,6 +1444,7 @@ getBBAddrMapFeature(const MachineFunction &MF, int NumMBBSectionRanges,
// Use static_cast to avoid breakage of tests on windows.
static_cast<bool>(BBAddrMapSkipEmitBBEntries), HasCalls,
static_cast<bool>(EmitBBHash)};
+ false};
}
void AsmPrinter::emitBBAddrMapSection(const MachineFunction &MF) {
diff --git a/llvm/lib/Object/ELF.cpp b/llvm/lib/Object/ELF.cpp
index 6da97f9b3755d..3ce096defc4fc 100644
--- a/llvm/lib/Object/ELF.cpp
+++ b/llvm/lib/Object/ELF.cpp
@@ -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
@@ -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.PropellerCfg && 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;
@@ -946,6 +951,10 @@ decodeBBAddrMapImpl(const ELFFile<ELFT> &EF,
uint64_t BBF = FeatEnable.BBFreq
? readULEB128As<uint64_t>(Data, Cur, ULEBSizeErr)
: 0;
+ uint32_t PropellerBBFreq =
+ FeatEnable.PropellerCfg
+ ? readULEB128As<uint32_t>(Data, Cur, ULEBSizeErr)
+ : 0;
// Branch probability
llvm::SmallVector<PGOAnalysisMap::PGOBBEntry::SuccessorEntry, 2>
@@ -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 PropellerFreq =
+ FeatEnable.PropellerCfg
+ ? readULEB128As<uint32_t>(Data, Cur, ULEBSizeErr)
+ : 0;
+
if (PGOAnalyses)
- Successors.push_back({BBID, BranchProbability::getRaw(BrProb)});
+ Successors.push_back(
+ {BBID, BranchProbability::getRaw(BrProb), PropellerFreq});
}
}
if (PGOAnalyses)
- PGOBBEntries.push_back({BlockFrequency(BBF), std::move(Successors)});
+ PGOBBEntries.push_back(
+ {BlockFrequency(BBF), PropellerBBFreq, std::move(Successors)});
}
if (PGOAnalyses)
diff --git a/llvm/lib/ObjectYAML/ELFEmitter.cpp b/llvm/lib/ObjectYAML/ELFEmitter.cpp
index 8b75fbe8291f0..19bbaa9de2871 100644
--- a/llvm/lib/ObjectYAML/ELFEmitter.cpp
+++ b/llvm/lib/ObjectYAML/ELFEmitter.cpp
@@ -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";
@@ -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->PropellerCfg || PGOBBE.PropellerBBFreq.has_value())
+ SHeader.sh_size += CBA.writeULEB128(PGOBBE.PropellerBBFreq.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, PropellerBrFreq] : *PGOBBE.Successors) {
SHeader.sh_size += CBA.writeULEB128(ID);
SHeader.sh_size += CBA.writeULEB128(BrProb);
+ if (FeatureOrErr->PropellerCfg || PropellerBrFreq.has_value())
+ SHeader.sh_size += CBA.writeULEB128(PropellerBrFreq.value_or(0));
}
}
}
diff --git a/llvm/lib/ObjectYAML/ELFYAML.cpp b/llvm/lib/ObjectYAML/ELFYAML.cpp
index f8a84b075b779..f0226a2444060 100644
--- a/llvm/lib/ObjectYAML/ELFYAML.cpp
+++ b/llvm/lib/ObjectYAML/ELFYAML.cpp
@@ -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("PropellerBBFreq", E.PropellerBBFreq);
IO.mapOptional("Successors", E.Successors);
}
@@ -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("PropellerBrFreq", E.PropellerBrFreq);
}
void MappingTraits<ELFYAML::GnuHashHeader>::mapping(IO &IO,
diff --git a/llvm/test/tools/llvm-readobj/ELF/bb-addr-map-pgo-analysis-map.test b/llvm/test/tools/llvm-readobj/ELF/bb-addr-map-pgo-analysis-map.test
index 5faafd4d83b2f..ca72a1e01a03c 100644
--- a/llvm/test/tools/llvm-readobj/ELF/bb-addr-map-pgo-analysis-map.test
+++ b/llvm/test/tools/llvm-readobj/ELF/bb-addr-map-pgo-analysis-map.test
@@ -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
@@ -59,17 +59,20 @@
# CHECK-NEXT: {
# RAW-NEXT: Frequency: 100
# PRETTY-NEXT: Frequency: 1.0
+# CHECK-NEXT: Propeller Frequency: 10
# CHECK-NEXT: Successors [
# CHECK-NEXT: {
# CHECK-NEXT: ID: 2
# RAW-NEXT: Probability: 0x80000000
# PRETTY-NEXT: Probability: 0x80000000 / 0x80000000 = 100.00%
+# CHECK-NEXT: Propeller Probability: 7
# CHECK-NEXT: }
# CHECK-NEXT: ]
# CHECK-NEXT: }
# CHECK-NEXT: {
# RAW-NEXT: Frequency: 100
# PRETTY-NEXT: Frequency: 1.0
+# CHECK-NEXT: Propeller Frequency: 0
# CHECK-NEXT: Successors [
# CHECK-NEXT: ]
# CHECK-NEXT: }
@@ -172,8 +175,8 @@ Sections:
ShSize: [[SIZE=<none>]]
Link: .text
Entries:
- - Version: 2
- Feature: 0x7
+ - Version: 5
+ Feature: 0x87
BBRanges:
- BaseAddress: [[ADDR=0x11111]]
BBEntries:
@@ -197,10 +200,12 @@ Sections:
PGOAnalyses:
- FuncEntryCount: 100
PGOBBEntries:
- - BBFreq: 100
+ - BBFreq: 100
+ PropellerBBFreq: 10
Successors:
- - ID: 2
- BrProb: 0x80000000
+ - ID: 2
+ BrProb: 0x80000000
+ PropellerBrFreq: 7
- BBFreq: 100
Successors: []
- FuncEntryCount: 8888
diff --git a/llvm/test/tools/obj2yaml/ELF/bb-addr-map-pgo-analysis-map.yaml b/llvm/test/tools/obj2yaml/ELF/bb-addr-map-pgo-analysis-map.yaml
index 299bf463cf4bc..3d7cc15f1f26f 100644
--- a/llvm/test/tools/obj2yaml/ELF/bb-addr-map-pgo-analysis-map.yaml
+++ b/llvm/test/tools/obj2yaml/ELF/bb-addr-map-pgo-analysis-map.yaml
@@ -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:
@@ -43,17 +43,23 @@
# VALID-NEXT: PGOAnalyses:
# VALID-NEXT: - FuncEntryCount: 100
# VALID-NEXT: PGOBBEntries:
-# VALID-NEXT: - BBFreq: 100
+# VALID-NEXT: - BBFreq: 100
+# VALID-NEXT: PropellerBBFreq: 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: PropellerBrFreq: 7
+# VALID-NEXT: - ID: 4
+# VALID-NEXT: BrProb: 0x80000000
+# VALID-NEXT: PropellerBrFreq: 0
+# VALID-NEXT: - BBFreq: 50
+# VALID-NEXT: PropellerBBFreq: 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: PropellerBrFreq: 0
+# VALID-NEXT: - BBFreq: 100
+# VALID-NEXT: PropellerBBFreq: 3
# VALID-NEXT: Successors: []
# VALID-NEXT: PGOBBEntries:
# VALID-NEXT: - BBFreq: 20
@@ -69,7 +75,7 @@ Sections:
ShSize: [[SIZE=<none>]]
Entries:
- Version: 2
- Feature: 0x7
+ Feature: 0x87
BBRanges:
- BaseAddress: 0x0
BBEntries:
@@ -97,17 +103,20 @@ Sections:
PGOAnalyses:
- FuncEntryCount: 100
PGOBBEntries:
- - BBFreq: 100
+ - BBFreq: 100
+ PropellerBBFreq: 10
Successors:
- - ID: 2
- BrProb: 0x80000000
- - ID: 4
- BrProb: 0x80000000
- - BBFreq: 50
+ - ID: 2
+ BrProb: 0x80000000
+ PropellerBrFreq: 7
+ - ID: 4
+ BrProb: 0x80000000
+ - BBFreq: 50
Successors:
- - ID: 4
- BrProb: 0xFFFFFFFF
- - BBFreq: 100
+ - ID: 4
+ BrProb: 0xFFFFFFFF
+ - BBFreq: 100
+ PropellerBBFreq: 3
Successors: []
- PGOBBEntries:
- BBFreq: 20
diff --git a/llvm/test/tools/yaml2obj/ELF/bb-addr-map-pgo-analysis-map.yaml b/llvm/test/tools/yaml2obj/ELF/bb-addr-map-pgo-analysis-map.yaml
index a4cb572e6d993..7f3c742b1d125 100644
--- a/llvm/test/tools/yaml2obj/ELF/bb-addr-map-pgo-analysis-map.yaml
+++ b/llvm/test/tools/yaml2obj/ELF/bb-addr-map-pgo-analysis-map.yaml
@@ -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
@@ -31,7 +32,7 @@ Sections:
Type: SHT_LLVM_BB_ADDR_MAP
Entries:
- Version: 2
- Feature: 0x7
+ Feature: 0x87
BBRanges:
- BaseAddress: 0x0000000000000020
BBEntries:
@@ -42,12 +43,14 @@ Sections:
PGOAnalyses:
- FuncEntryCount: 1000
PGOBBEntries:
- - BBFreq: 1000
+ - BBFreq: 1000
+ PropellerBBFreq: 100
Successors:
- - ID: 12
- BrProb: 0xeeeeeeee
- - ID: 13
- BrProb: 0x11111111
+ - ID: 12
+ BrProb: 0xeeeeeeee
+ PropellerBrFreq: 40
+ - ID: 13
+ BrProb: 0x11111111
## 2) According to feature we have FuncEntryCount but none is provided in yaml
- Name: '.llvm_bb_addr_map (2)'
@@ -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
--- !ELF
FileHeader:
@@ -79,4 +82,4 @@ Sections:
Entries:
- Version: 2
## Specify unsupported feature
- Feature: 0xF0
+ Feature: 0x100
diff --git a/llvm/test/tools/yaml2obj/ELF/bb-addr-map.yaml b/llvm/test/tools/yaml2obj/ELF/bb-addr-map.yaml
index 339e419b39458..05d77d67e4468 100644
--- a/llvm/test/tools/yaml2obj/ELF/bb-addr-map.yaml
+++ b/llvm/test/tools/yaml2obj/ELF/bb-addr-map.yaml
@@ -220,7 +220,7 @@ Sections:
## Check that yaml2obj generates a warning when we use unsupported versions.
# RUN: yaml2obj --docnum=3 %s 2>&1 | FileCheck %s --check-prefix=INVALID-VERSION
-# INVALID-VERSION: warning: unsupported SHT_LLVM_BB_ADDR_MAP version: 5; encoding using the most recent version
+# INVALID-VERSION: warning: unsupported SHT_LLVM_BB_ADDR_MAP version: 6; encoding using the most recent version
--- !ELF
FileHeader:
@@ -232,4 +232,4 @@ Sections:
Type: SHT_LLVM_BB_ADDR_MAP
Entries:
## Specify unsupported version
- - Version: 5
+ - Version: 6
diff --git a/llvm/tools/llvm-readobj/ELFDumper.cpp b/llvm/tools/llvm-readobj/ELFDumper.cpp
index 9c9b2dd79e686..4067125069d9e 100644
--- a/llvm/tools/llvm-readobj/ELFDumper.cpp
+++ b/llvm/tools/llvm-readobj/ELFDumper.cpp
@@ -8188,6 +8188,8 @@ void LLVMELFDumper<ELFT>::printBBAddrMaps(bool PrettyPGOAnalysis) {
} else {
W.printNumber("Frequency", PBBE.BlockFreq.getFrequency());
}
+ if (PAM.FeatEnable.PropellerCfg)
+ W.printNumber("Propeller Frequency", PBBE.PropellerBlockFreq);
}
if (PAM.FeatEnable.BrProb) {
@@ -8200,6 +8202,8 @@ void LLVMELFDumper<ELFT>::printBBAddrMaps(bool PrettyPGOAnalysis) {
} else {
W.printHex("Probability", Succ.Prob.getNumerator());
}
+ if (PAM.FeatEnable.PropellerCfg)
+ W.printNumber("Propeller Probability", Succ.PropellerFreq);
}
}
}
diff --git a/llvm/tools/obj2yaml/elf2yaml.cpp b/llvm/tools/obj2yaml/elf2yaml.cpp
index 68e18f6c79202..9e32a294554b5 100644
--- a/llvm/tools/obj2yaml/elf2yaml.cpp
+++ b/llvm/tools/obj2yaml/elf2yaml.cpp
@@ -972,6 +972,8 @@ ELFDumper<ELFT>::dumpBBAddrMapSection(const Elf_Shdr *Shdr) {
auto &PGOBBEntry = PGOBBEntries.emplace_back();
if (FeatureOrErr->BBFreq) {
PGOBBEntry.BBFreq = Data.getULEB128(Cur);
+ if (FeatureOrErr->PropellerCfg)
+ PGOBBEntry.PropellerBBFreq = Data.getULEB128(Cur);
if (!Cur)
break;
}
@@ -982,7 +984,10 @@ ELFDumper<ELFT>::dumpBBAddrMapSection(const Elf_Shdr *Shdr) {
for (uint64_t SuccIdx = 0; Cur && SuccIdx < SuccCount; ++SuccIdx) {
ui...
[truncated]
|
|
@llvm/pr-subscribers-objectyaml Author: Rahman Lavaee (rlavaee) ChangesThis PR implements the ELF support for Propeller CFG in PGO analysis map as discussed in RFC. Patch is 20.40 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/164914.diff 12 Files Affected:
diff --git a/llvm/include/llvm/Object/ELFTypes.h b/llvm/include/llvm/Object/ELFTypes.h
index e9a417d3d4fb3..101ead91df4bd 100644
--- a/llvm/include/llvm/Object/ELFTypes.h
+++ b/llvm/include/llvm/Object/ELFTypes.h
@@ -834,6 +834,7 @@ struct BBAddrMap {
bool OmitBBEntries : 1;
bool CallsiteEndOffsets : 1;
bool BBHash : 1;
+ bool PropellerCfg : 1;
bool hasPGOAnalysis() const { return FuncEntryCount || BBFreq || BrProb; }
@@ -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>(PropellerCfg) << 7);
}
// Decodes from minimum bit width representation and validates no
@@ -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",
@@ -867,10 +869,12 @@ struct BBAddrMap {
bool operator==(const Features &Other) const {
return std::tie(FuncEntryCount, BBFreq, BrProb, MultiBBRange,
- OmitBBEntries, CallsiteEndOffsets, BBHash) ==
+ OmitBBEntries, CallsiteEndOffsets, BBHash,
+ PropellerCfg) ==
std::tie(Other.FuncEntryCount, Other.BBFreq, Other.BrProb,
Other.MultiBBRange, Other.OmitBBEntries,
- Other.CallsiteEndOffsets, Other.BBHash);
+ Other.CallsiteEndOffsets, Other.BBHash,
+ Other.PropellerCfg);
}
};
@@ -1013,20 +1017,25 @@ struct PGOAnalysisMap {
uint32_t ID;
/// Branch Probability of the edge to this successor taken from MBPI.
BranchProbability Prob;
+ /// Edge frequency from Propeller.
+ uint32_t PropellerFreq;
bool operator==(const SuccessorEntry &Other) const {
- return std::tie(ID, Prob) == std::tie(Other.ID, Other.Prob);
+ return std::tie(ID, Prob, PropellerFreq) ==
+ std::tie(Other.ID, Other.Prob, Other.PropellerFreq);
}
};
/// Block frequency taken from MBFI
BlockFrequency BlockFreq;
+ /// Block frequency taken from Propeller.
+ uint32_t PropellerBlockFreq;
/// 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, PropellerBlockFreq, Successors) ==
+ std::tie(Other.BlockFreq, PropellerBlockFreq, Other.Successors);
}
};
diff --git a/llvm/include/llvm/ObjectYAML/ELFYAML.h b/llvm/include/llvm/ObjectYAML/ELFYAML.h
index a7c7c7c436dc2..1b7a4018369c3 100644
--- a/llvm/include/llvm/ObjectYAML/ELFYAML.h
+++ b/llvm/include/llvm/ObjectYAML/ELFYAML.h
@@ -203,8 +203,10 @@ struct PGOAnalysisMapEntry {
struct SuccessorEntry {
uint32_t ID;
llvm::yaml::Hex32 BrProb;
+ std::optional<uint32_t> PropellerBrFreq;
};
std::optional<uint64_t> BBFreq;
+ std::optional<uint32_t> PropellerBBFreq;
std::optional<std::vector<SuccessorEntry>> Successors;
};
std::optional<uint64_t> FuncEntryCount;
diff --git a/llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp b/llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp
index 8aa488f0efd8f..8f000dbba327a 100644
--- a/llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp
+++ b/llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp
@@ -1444,6 +1444,7 @@ getBBAddrMapFeature(const MachineFunction &MF, int NumMBBSectionRanges,
// Use static_cast to avoid breakage of tests on windows.
static_cast<bool>(BBAddrMapSkipEmitBBEntries), HasCalls,
static_cast<bool>(EmitBBHash)};
+ false};
}
void AsmPrinter::emitBBAddrMapSection(const MachineFunction &MF) {
diff --git a/llvm/lib/Object/ELF.cpp b/llvm/lib/Object/ELF.cpp
index 6da97f9b3755d..3ce096defc4fc 100644
--- a/llvm/lib/Object/ELF.cpp
+++ b/llvm/lib/Object/ELF.cpp
@@ -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
@@ -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.PropellerCfg && 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;
@@ -946,6 +951,10 @@ decodeBBAddrMapImpl(const ELFFile<ELFT> &EF,
uint64_t BBF = FeatEnable.BBFreq
? readULEB128As<uint64_t>(Data, Cur, ULEBSizeErr)
: 0;
+ uint32_t PropellerBBFreq =
+ FeatEnable.PropellerCfg
+ ? readULEB128As<uint32_t>(Data, Cur, ULEBSizeErr)
+ : 0;
// Branch probability
llvm::SmallVector<PGOAnalysisMap::PGOBBEntry::SuccessorEntry, 2>
@@ -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 PropellerFreq =
+ FeatEnable.PropellerCfg
+ ? readULEB128As<uint32_t>(Data, Cur, ULEBSizeErr)
+ : 0;
+
if (PGOAnalyses)
- Successors.push_back({BBID, BranchProbability::getRaw(BrProb)});
+ Successors.push_back(
+ {BBID, BranchProbability::getRaw(BrProb), PropellerFreq});
}
}
if (PGOAnalyses)
- PGOBBEntries.push_back({BlockFrequency(BBF), std::move(Successors)});
+ PGOBBEntries.push_back(
+ {BlockFrequency(BBF), PropellerBBFreq, std::move(Successors)});
}
if (PGOAnalyses)
diff --git a/llvm/lib/ObjectYAML/ELFEmitter.cpp b/llvm/lib/ObjectYAML/ELFEmitter.cpp
index 8b75fbe8291f0..19bbaa9de2871 100644
--- a/llvm/lib/ObjectYAML/ELFEmitter.cpp
+++ b/llvm/lib/ObjectYAML/ELFEmitter.cpp
@@ -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";
@@ -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->PropellerCfg || PGOBBE.PropellerBBFreq.has_value())
+ SHeader.sh_size += CBA.writeULEB128(PGOBBE.PropellerBBFreq.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, PropellerBrFreq] : *PGOBBE.Successors) {
SHeader.sh_size += CBA.writeULEB128(ID);
SHeader.sh_size += CBA.writeULEB128(BrProb);
+ if (FeatureOrErr->PropellerCfg || PropellerBrFreq.has_value())
+ SHeader.sh_size += CBA.writeULEB128(PropellerBrFreq.value_or(0));
}
}
}
diff --git a/llvm/lib/ObjectYAML/ELFYAML.cpp b/llvm/lib/ObjectYAML/ELFYAML.cpp
index f8a84b075b779..f0226a2444060 100644
--- a/llvm/lib/ObjectYAML/ELFYAML.cpp
+++ b/llvm/lib/ObjectYAML/ELFYAML.cpp
@@ -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("PropellerBBFreq", E.PropellerBBFreq);
IO.mapOptional("Successors", E.Successors);
}
@@ -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("PropellerBrFreq", E.PropellerBrFreq);
}
void MappingTraits<ELFYAML::GnuHashHeader>::mapping(IO &IO,
diff --git a/llvm/test/tools/llvm-readobj/ELF/bb-addr-map-pgo-analysis-map.test b/llvm/test/tools/llvm-readobj/ELF/bb-addr-map-pgo-analysis-map.test
index 5faafd4d83b2f..ca72a1e01a03c 100644
--- a/llvm/test/tools/llvm-readobj/ELF/bb-addr-map-pgo-analysis-map.test
+++ b/llvm/test/tools/llvm-readobj/ELF/bb-addr-map-pgo-analysis-map.test
@@ -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
@@ -59,17 +59,20 @@
# CHECK-NEXT: {
# RAW-NEXT: Frequency: 100
# PRETTY-NEXT: Frequency: 1.0
+# CHECK-NEXT: Propeller Frequency: 10
# CHECK-NEXT: Successors [
# CHECK-NEXT: {
# CHECK-NEXT: ID: 2
# RAW-NEXT: Probability: 0x80000000
# PRETTY-NEXT: Probability: 0x80000000 / 0x80000000 = 100.00%
+# CHECK-NEXT: Propeller Probability: 7
# CHECK-NEXT: }
# CHECK-NEXT: ]
# CHECK-NEXT: }
# CHECK-NEXT: {
# RAW-NEXT: Frequency: 100
# PRETTY-NEXT: Frequency: 1.0
+# CHECK-NEXT: Propeller Frequency: 0
# CHECK-NEXT: Successors [
# CHECK-NEXT: ]
# CHECK-NEXT: }
@@ -172,8 +175,8 @@ Sections:
ShSize: [[SIZE=<none>]]
Link: .text
Entries:
- - Version: 2
- Feature: 0x7
+ - Version: 5
+ Feature: 0x87
BBRanges:
- BaseAddress: [[ADDR=0x11111]]
BBEntries:
@@ -197,10 +200,12 @@ Sections:
PGOAnalyses:
- FuncEntryCount: 100
PGOBBEntries:
- - BBFreq: 100
+ - BBFreq: 100
+ PropellerBBFreq: 10
Successors:
- - ID: 2
- BrProb: 0x80000000
+ - ID: 2
+ BrProb: 0x80000000
+ PropellerBrFreq: 7
- BBFreq: 100
Successors: []
- FuncEntryCount: 8888
diff --git a/llvm/test/tools/obj2yaml/ELF/bb-addr-map-pgo-analysis-map.yaml b/llvm/test/tools/obj2yaml/ELF/bb-addr-map-pgo-analysis-map.yaml
index 299bf463cf4bc..3d7cc15f1f26f 100644
--- a/llvm/test/tools/obj2yaml/ELF/bb-addr-map-pgo-analysis-map.yaml
+++ b/llvm/test/tools/obj2yaml/ELF/bb-addr-map-pgo-analysis-map.yaml
@@ -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:
@@ -43,17 +43,23 @@
# VALID-NEXT: PGOAnalyses:
# VALID-NEXT: - FuncEntryCount: 100
# VALID-NEXT: PGOBBEntries:
-# VALID-NEXT: - BBFreq: 100
+# VALID-NEXT: - BBFreq: 100
+# VALID-NEXT: PropellerBBFreq: 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: PropellerBrFreq: 7
+# VALID-NEXT: - ID: 4
+# VALID-NEXT: BrProb: 0x80000000
+# VALID-NEXT: PropellerBrFreq: 0
+# VALID-NEXT: - BBFreq: 50
+# VALID-NEXT: PropellerBBFreq: 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: PropellerBrFreq: 0
+# VALID-NEXT: - BBFreq: 100
+# VALID-NEXT: PropellerBBFreq: 3
# VALID-NEXT: Successors: []
# VALID-NEXT: PGOBBEntries:
# VALID-NEXT: - BBFreq: 20
@@ -69,7 +75,7 @@ Sections:
ShSize: [[SIZE=<none>]]
Entries:
- Version: 2
- Feature: 0x7
+ Feature: 0x87
BBRanges:
- BaseAddress: 0x0
BBEntries:
@@ -97,17 +103,20 @@ Sections:
PGOAnalyses:
- FuncEntryCount: 100
PGOBBEntries:
- - BBFreq: 100
+ - BBFreq: 100
+ PropellerBBFreq: 10
Successors:
- - ID: 2
- BrProb: 0x80000000
- - ID: 4
- BrProb: 0x80000000
- - BBFreq: 50
+ - ID: 2
+ BrProb: 0x80000000
+ PropellerBrFreq: 7
+ - ID: 4
+ BrProb: 0x80000000
+ - BBFreq: 50
Successors:
- - ID: 4
- BrProb: 0xFFFFFFFF
- - BBFreq: 100
+ - ID: 4
+ BrProb: 0xFFFFFFFF
+ - BBFreq: 100
+ PropellerBBFreq: 3
Successors: []
- PGOBBEntries:
- BBFreq: 20
diff --git a/llvm/test/tools/yaml2obj/ELF/bb-addr-map-pgo-analysis-map.yaml b/llvm/test/tools/yaml2obj/ELF/bb-addr-map-pgo-analysis-map.yaml
index a4cb572e6d993..7f3c742b1d125 100644
--- a/llvm/test/tools/yaml2obj/ELF/bb-addr-map-pgo-analysis-map.yaml
+++ b/llvm/test/tools/yaml2obj/ELF/bb-addr-map-pgo-analysis-map.yaml
@@ -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
@@ -31,7 +32,7 @@ Sections:
Type: SHT_LLVM_BB_ADDR_MAP
Entries:
- Version: 2
- Feature: 0x7
+ Feature: 0x87
BBRanges:
- BaseAddress: 0x0000000000000020
BBEntries:
@@ -42,12 +43,14 @@ Sections:
PGOAnalyses:
- FuncEntryCount: 1000
PGOBBEntries:
- - BBFreq: 1000
+ - BBFreq: 1000
+ PropellerBBFreq: 100
Successors:
- - ID: 12
- BrProb: 0xeeeeeeee
- - ID: 13
- BrProb: 0x11111111
+ - ID: 12
+ BrProb: 0xeeeeeeee
+ PropellerBrFreq: 40
+ - ID: 13
+ BrProb: 0x11111111
## 2) According to feature we have FuncEntryCount but none is provided in yaml
- Name: '.llvm_bb_addr_map (2)'
@@ -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
--- !ELF
FileHeader:
@@ -79,4 +82,4 @@ Sections:
Entries:
- Version: 2
## Specify unsupported feature
- Feature: 0xF0
+ Feature: 0x100
diff --git a/llvm/test/tools/yaml2obj/ELF/bb-addr-map.yaml b/llvm/test/tools/yaml2obj/ELF/bb-addr-map.yaml
index 339e419b39458..05d77d67e4468 100644
--- a/llvm/test/tools/yaml2obj/ELF/bb-addr-map.yaml
+++ b/llvm/test/tools/yaml2obj/ELF/bb-addr-map.yaml
@@ -220,7 +220,7 @@ Sections:
## Check that yaml2obj generates a warning when we use unsupported versions.
# RUN: yaml2obj --docnum=3 %s 2>&1 | FileCheck %s --check-prefix=INVALID-VERSION
-# INVALID-VERSION: warning: unsupported SHT_LLVM_BB_ADDR_MAP version: 5; encoding using the most recent version
+# INVALID-VERSION: warning: unsupported SHT_LLVM_BB_ADDR_MAP version: 6; encoding using the most recent version
--- !ELF
FileHeader:
@@ -232,4 +232,4 @@ Sections:
Type: SHT_LLVM_BB_ADDR_MAP
Entries:
## Specify unsupported version
- - Version: 5
+ - Version: 6
diff --git a/llvm/tools/llvm-readobj/ELFDumper.cpp b/llvm/tools/llvm-readobj/ELFDumper.cpp
index 9c9b2dd79e686..4067125069d9e 100644
--- a/llvm/tools/llvm-readobj/ELFDumper.cpp
+++ b/llvm/tools/llvm-readobj/ELFDumper.cpp
@@ -8188,6 +8188,8 @@ void LLVMELFDumper<ELFT>::printBBAddrMaps(bool PrettyPGOAnalysis) {
} else {
W.printNumber("Frequency", PBBE.BlockFreq.getFrequency());
}
+ if (PAM.FeatEnable.PropellerCfg)
+ W.printNumber("Propeller Frequency", PBBE.PropellerBlockFreq);
}
if (PAM.FeatEnable.BrProb) {
@@ -8200,6 +8202,8 @@ void LLVMELFDumper<ELFT>::printBBAddrMaps(bool PrettyPGOAnalysis) {
} else {
W.printHex("Probability", Succ.Prob.getNumerator());
}
+ if (PAM.FeatEnable.PropellerCfg)
+ W.printNumber("Propeller Probability", Succ.PropellerFreq);
}
}
}
diff --git a/llvm/tools/obj2yaml/elf2yaml.cpp b/llvm/tools/obj2yaml/elf2yaml.cpp
index 68e18f6c79202..9e32a294554b5 100644
--- a/llvm/tools/obj2yaml/elf2yaml.cpp
+++ b/llvm/tools/obj2yaml/elf2yaml.cpp
@@ -972,6 +972,8 @@ ELFDumper<ELFT>::dumpBBAddrMapSection(const Elf_Shdr *Shdr) {
auto &PGOBBEntry = PGOBBEntries.emplace_back();
if (FeatureOrErr->BBFreq) {
PGOBBEntry.BBFreq = Data.getULEB128(Cur);
+ if (FeatureOrErr->PropellerCfg)
+ PGOBBEntry.PropellerBBFreq = Data.getULEB128(Cur);
if (!Cur)
break;
}
@@ -982,7 +984,10 @@ ELFDumper<ELFT>::dumpBBAddrMapSection(const Elf_Shdr *Shdr) {
for (uint64_t SuccIdx = 0; Cur && SuccIdx < SuccCount; ++SuccIdx) {
ui...
[truncated]
|
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
mtrofin
reviewed
Oct 27, 2025
llvm/include/llvm/Object/ELFTypes.h
Outdated
| /// probability associated with it. | ||
| struct SuccessorEntry { | ||
| /// Unique ID of this successor basic block. | ||
| uint32_t ID; |
There was a problem hiding this comment.
can you zero-init the primitive types? less chance of UB through maintenance later
mtrofin
reviewed
Oct 27, 2025
llvm/include/llvm/Object/ELFTypes.h
Outdated
| /// Block frequency taken from MBFI | ||
| BlockFrequency BlockFreq; | ||
| /// Block frequency taken from Propeller. | ||
| uint32_t PropellerBlockFreq; |
There was a problem hiding this comment.
can the comment explain how this is to be used? is it like BFI, where you need to make it relative to the entry BB?
why uint32_t and not uint64_t?
|
I don't have any especially major concerns with this. I guess my biggest concern is that I don't think it's right to have references to non-LLVM tools in the LLVM-specific ELF data structures. This is more a naming concern than anything else - if you can come up with a generic name, I'd be less concerned. If we were to allow this change as-is, another tool might come along in the future that uses a similar design but we'd have to rename everything or not realise that there's an opportunity to reuse the field being introduced here. Furthermore, it would leave us open to the argument that we need to support every tool under the sun that could generate even remotely related data, with its own specific set of fields, which isn't really sustainable. That does raise the question as to whether this approach is the right approach at all and whether instead we should have some sort of generically extendable section that individual tools can leverage. I don't have any concrete ideas here currently though. |
rlavaee
force-pushed
the
propeller-cfg-decode
branch
from
2e902c2 to
3832bf9
Compare
Fair point, as we discussed in the RFC, this can be used for any post link profiles. So I went ahead and renamed it to PostLink. |
mtrofin
approved these changes
Oct 29, 2025
jh7370
reviewed
Oct 29, 2025
Comment on lines
70
to
72
| ## 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 |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
jh7370
approved these changes
Oct 30, 2025
llvm/include/llvm/Object/ELFTypes.h
Outdated
| // Encodes to minimum bit width representation. | ||
| uint8_t encode() const { | ||
| uint16_t encode() const { | ||
| return (static_cast<uint8_t>(FuncEntryCount) << 0) | |
There was a problem hiding this comment.
Probably worth changing these to uint16_t at the same time, or risk a copy/paste issue when the next feature is introduced.
Comment on lines
11
to
12
|
|
||
|
|
There was a problem hiding this comment.
Nit: more than one blank line here and below is excessive.
boomanaiden154
approved these changes
Oct 30, 2025
luciechoi
pushed a commit
to luciechoi/llvm-project
that referenced
this pull request
Nov 1, 2025
…ata in PGO analysis map. (llvm#164914) This PR implements the ELF support for PostLink CFG in PGO analysis map as discussed in [RFC](https://discourse.llvm.org/t/rfc-extending-the-pgo-analysis-map-with-propeller-cfg-frequencies/88617/2). A later PR will implement the Codegen Support.
DEBADRIBASAK
pushed a commit
to DEBADRIBASAK/llvm-project
that referenced
this pull request
Nov 3, 2025
…ata in PGO analysis map. (llvm#164914) This PR implements the ELF support for PostLink CFG in PGO analysis map as discussed in [RFC](https://discourse.llvm.org/t/rfc-extending-the-pgo-analysis-map-with-propeller-cfg-frequencies/88617/2). A later PR will implement the Codegen Support.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
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.
This PR implements the ELF support for PostLink CFG in PGO analysis map as discussed in RFC.
A later PR will implement the Codegen Support.