-
Notifications
You must be signed in to change notification settings - Fork 14.3k
[SHT_LLVM_FUNC_MAP][ObjectYaml]Introduce function address map section and emit dynamic instruction count(ObjectYaml part) #124332
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from 1 commit
2e0f4ff
d2b1895
5841112
cc11306
e8a1dce
d2d627e
970042e
3703e53
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -1027,6 +1027,44 @@ struct PGOAnalysisMap { | |||||
} | ||||||
}; | ||||||
|
||||||
// Struct representing the FuncMap for one function. | ||||||
struct FuncMap { | ||||||
|
||||||
// Bitfield of optional features to control the extra information | ||||||
// emitted/encoded in the the section. | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
struct Features { | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do we really need this feature bitfield at all if there's only one feature and a version for the section? It feels redundant to me. If the section needs extending e.g. to add more features, you'll presumably need to update the version number anyway, in which case you can add optional feature toggles as needed at that point. Otherwise, this just feels like unnecessary complexity and wasted space in the encoding. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. +1. Let's add this Features bitfield once it's really needed. We can keep everything simple for now. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sounds good, I will remove the |
||||||
bool DynamicInstCount : 1; | ||||||
|
||||||
// Encodes to minimum bit width representation. | ||||||
uint8_t encode() const { | ||||||
return (static_cast<uint8_t>(DynamicInstCount) << 0); | ||||||
} | ||||||
|
||||||
// Decodes from minimum bit width representation and validates no | ||||||
// unnecessary bits are used. | ||||||
static Expected<Features> decode(uint8_t Val) { | ||||||
Features Feat{static_cast<bool>(Val & (1 << 0))}; | ||||||
if (Feat.encode() != Val) | ||||||
return createStringError(std::error_code(), | ||||||
"invalid encoding for FuncMap::Features: 0x%x", | ||||||
Val); | ||||||
return Feat; | ||||||
} | ||||||
|
||||||
bool operator==(const Features &Other) const { | ||||||
return DynamicInstCount == Other.DynamicInstCount; | ||||||
} | ||||||
}; | ||||||
|
||||||
uint64_t FunctionAddress = 0; // Function entry address. | ||||||
uint64_t DynamicInstCount = 0; // Dynamic instruction count for this function | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nit: don't forget to add "." at the end of each comment. Applies here and below. |
||||||
|
||||||
// Flags to indicate if each feature was enabled in this function | ||||||
Features FeatEnable; | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||||||
|
||||||
uint64_t getFunctionAddress() const { return FunctionAddress; } | ||||||
}; | ||||||
|
||||||
} // end namespace object. | ||||||
} // end namespace llvm. | ||||||
|
||||||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -195,6 +195,13 @@ struct PGOAnalysisMapEntry { | |
std::optional<std::vector<PGOBBEntry>> PGOBBEntries; | ||
}; | ||
|
||
struct FuncMapEntry { | ||
uint8_t Version; | ||
llvm::yaml::Hex8 Feature; | ||
llvm::yaml::Hex64 Address; | ||
llvm::yaml::Hex64 DynamicInstCount; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This can be a plain integer. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. changed to |
||
}; | ||
|
||
struct StackSizeEntry { | ||
llvm::yaml::Hex64 Address; | ||
llvm::yaml::Hex64 Size; | ||
|
@@ -229,6 +236,7 @@ struct Chunk { | |
DependentLibraries, | ||
CallGraphProfile, | ||
BBAddrMap, | ||
FuncMap, | ||
|
||
// Special chunks. | ||
SpecialChunksStart, | ||
|
@@ -355,6 +363,18 @@ struct BBAddrMapSection : Section { | |
} | ||
}; | ||
|
||
struct FuncMapSection : Section { | ||
std::optional<std::vector<FuncMapEntry>> Entries; | ||
|
||
FuncMapSection() : Section(ChunkKind::FuncMap) {} | ||
|
||
std::vector<std::pair<StringRef, bool>> getEntries() const override { | ||
return {{"Entries", Entries.has_value()}}; | ||
}; | ||
|
||
static bool classof(const Chunk *S) { return S->Kind == ChunkKind::FuncMap; } | ||
}; | ||
|
||
struct StackSizesSection : Section { | ||
std::optional<std::vector<StackSizeEntry>> Entries; | ||
|
||
|
@@ -762,6 +782,7 @@ bool shouldAllocateFileSpace(ArrayRef<ProgramHeader> Phdrs, | |
} // end namespace llvm | ||
|
||
LLVM_YAML_IS_SEQUENCE_VECTOR(llvm::ELFYAML::StackSizeEntry) | ||
LLVM_YAML_IS_SEQUENCE_VECTOR(llvm::ELFYAML::FuncMapEntry) | ||
LLVM_YAML_IS_SEQUENCE_VECTOR(llvm::ELFYAML::BBAddrMapEntry) | ||
LLVM_YAML_IS_SEQUENCE_VECTOR(llvm::ELFYAML::BBAddrMapEntry::BBEntry) | ||
LLVM_YAML_IS_SEQUENCE_VECTOR(llvm::ELFYAML::BBAddrMapEntry::BBRangeEntry) | ||
|
@@ -929,6 +950,10 @@ template <> struct MappingTraits<ELFYAML::StackSizeEntry> { | |
static void mapping(IO &IO, ELFYAML::StackSizeEntry &Rel); | ||
}; | ||
|
||
template <> struct MappingTraits<ELFYAML::FuncMapEntry> { | ||
static void mapping(IO &IO, ELFYAML::FuncMapEntry &E); | ||
}; | ||
|
||
template <> struct MappingTraits<ELFYAML::BBAddrMapEntry> { | ||
static void mapping(IO &IO, ELFYAML::BBAddrMapEntry &E); | ||
}; | ||
|
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -287,6 +287,9 @@ template <class ELFT> class ELFState { | |||||
void writeSectionContent(Elf_Shdr &SHeader, | ||||||
const ELFYAML::BBAddrMapSection &Section, | ||||||
ContiguousBlobAccumulator &CBA); | ||||||
void writeSectionContent(Elf_Shdr &SHeader, | ||||||
const ELFYAML::FuncMapSection &Section, | ||||||
ContiguousBlobAccumulator &CBA); | ||||||
void writeSectionContent(Elf_Shdr &SHeader, | ||||||
const ELFYAML::HashSection &Section, | ||||||
ContiguousBlobAccumulator &CBA); | ||||||
|
@@ -894,6 +897,8 @@ void ELFState<ELFT>::initSectionHeaders(std::vector<Elf_Shdr> &SHeaders, | |||||
writeSectionContent(SHeader, *S, CBA); | ||||||
} else if (auto S = dyn_cast<ELFYAML::BBAddrMapSection>(Sec)) { | ||||||
writeSectionContent(SHeader, *S, CBA); | ||||||
} else if (auto S = dyn_cast<ELFYAML::FuncMapSection>(Sec)) { | ||||||
writeSectionContent(SHeader, *S, CBA); | ||||||
} else { | ||||||
llvm_unreachable("Unknown section type"); | ||||||
} | ||||||
|
@@ -1537,6 +1542,31 @@ void ELFState<ELFT>::writeSectionContent( | |||||
} | ||||||
} | ||||||
|
||||||
template <class ELFT> | ||||||
void ELFState<ELFT>::writeSectionContent(Elf_Shdr &SHeader, | ||||||
const ELFYAML::FuncMapSection &Section, | ||||||
ContiguousBlobAccumulator &CBA) { | ||||||
if (!Section.Entries) | ||||||
return; | ||||||
|
||||||
for (const auto &[Idx, E] : llvm::enumerate(*Section.Entries)) { | ||||||
// Write version and feature values. | ||||||
if (Section.Type == llvm::ELF::SHT_LLVM_FUNC_MAP) { | ||||||
if (E.Version > 1) | ||||||
WithColor::warning() << "unsupported SHT_LLVM_FUNC_MAP version: " | ||||||
<< static_cast<int>(E.Version) | ||||||
<< "; encoding using the most recent version"; | ||||||
CBA.write(E.Version); | ||||||
CBA.write(E.Feature); | ||||||
SHeader.sh_size += 2; | ||||||
} | ||||||
CBA.write<uintX_t>(E.Address, ELFT::Endianness); | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Correct me if I'm wrong, but shouldn't this be using the Elf_Addr type, since it's representing an address? They might amount to the same thing, but it conveys the meaning better. (NB: I haven't tested it, so this might not work as desired)
Suggested change
|
||||||
SHeader.sh_size += sizeof(uintX_t); | ||||||
if (E.DynamicInstCount) | ||||||
SHeader.sh_size += CBA.writeULEB128(E.DynamicInstCount); | ||||||
} | ||||||
} | ||||||
|
||||||
template <class ELFT> | ||||||
void ELFState<ELFT>::writeSectionContent( | ||||||
Elf_Shdr &SHeader, const ELFYAML::LinkerOptionsSection &Section, | ||||||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,139 @@ | ||
## Check how obj2yaml produces YAML .llvm_func_map descriptions. | ||
|
||
## Check that obj2yaml uses the "Entries" tag to describe an .llvm_func_map section. | ||
|
||
# RUN: yaml2obj --docnum=1 %s -o %t1 | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You probably should have a 32-bit version of this test case too, since you're writing an address, which has a different size on 32-bit architectures. |
||
# RUN: obj2yaml %t1 | FileCheck %s --check-prefix=VALID | ||
|
||
# VALID: --- !ELF | ||
# VALID-NEXT: FileHeader: | ||
# VALID-NEXT: Class: ELFCLASS64 | ||
# VALID-NEXT: Data: ELFDATA2LSB | ||
# VALID-NEXT: Type: ET_EXEC | ||
# VALID-NEXT: Sections: | ||
# VALID-NEXT: - Name: .llvm_func_map | ||
# VALID-NEXT: Type: SHT_LLVM_FUNC_MAP | ||
# VALID-NEXT: Entries: | ||
# VALID-NEXT: - Version: 1 | ||
# VALID-NEXT: Feature: 0x1 | ||
## The 'Address' field is omitted when it's zero. | ||
# VALID-NEXT: DynInstCnt: 0x10 | ||
# VALID-NEXT: - Version: 1 | ||
## The 'Feature' field is omitted when it's zero. | ||
# VALID-NEXT: Address: 0x1 | ||
# VALID-NEXT: - Version: 1 | ||
# VALID-NEXT: Feature: 0x1 | ||
# VALID-NEXT: Address: 0xFFFFFFFFFFFFFFF1 | ||
# VALID-NEXT: DynInstCnt: 0xFFFFFFFFFFFFFFF2 | ||
|
||
--- !ELF | ||
FileHeader: | ||
Class: ELFCLASS64 | ||
Data: ELFDATA2LSB | ||
Type: ET_EXEC | ||
Sections: | ||
- Name: .llvm_func_map | ||
Type: SHT_LLVM_FUNC_MAP | ||
ShSize: [[SIZE=<none>]] | ||
Entries: | ||
- Version: 1 | ||
Feature: 0x1 | ||
Address: 0x0 | ||
DynInstCnt: 0x10 | ||
- Version: 1 | ||
Feature: 0x0 | ||
Address: 0x1 | ||
- Version: 1 | ||
Feature: 0x1 | ||
Address: 0xFFFFFFFFFFFFFFF1 | ||
DynInstCnt: 0xFFFFFFFFFFFFFFF2 | ||
|
||
## Check obj2yaml can dump empty .llvm_func_map sections. | ||
|
||
# RUN: yaml2obj --docnum=2 %s -o %t2 | ||
# RUN: obj2yaml %t2 | FileCheck %s --check-prefix=EMPTY | ||
|
||
# EMPTY: --- !ELF | ||
# EMPTY-NEXT: FileHeader: | ||
# EMPTY-NEXT: Class: ELFCLASS64 | ||
# EMPTY-NEXT: Data: ELFDATA2LSB | ||
# EMPTY-NEXT: Type: ET_EXEC | ||
# EMPTY-NEXT: Sections: | ||
# EMPTY-NEXT: - Name: .llvm_func_map | ||
# EMPTY-NEXT: Type: SHT_LLVM_FUNC_MAP | ||
# EMPTY-NOT: Content: | ||
|
||
--- !ELF | ||
FileHeader: | ||
Class: ELFCLASS64 | ||
Data: ELFDATA2LSB | ||
Type: ET_EXEC | ||
Sections: | ||
- Name: .llvm_func_map | ||
Type: SHT_LLVM_FUNC_MAP | ||
Content: "" | ||
|
||
## Check obj2yaml can dump multiple .llvm_func_map sections. | ||
|
||
# RUN: yaml2obj --docnum=3 %s -o %t3 | ||
# RUN: obj2yaml %t3 | FileCheck %s --check-prefix=MULTI | ||
|
||
# MULTI: --- !ELF | ||
# MULTI-NEXT: FileHeader: | ||
# MULTI-NEXT: Class: ELFCLASS64 | ||
# MULTI-NEXT: Data: ELFDATA2LSB | ||
# MULTI-NEXT: Type: ET_EXEC | ||
# MULTI-NEXT: Sections: | ||
# MULTI-NEXT: - Name: .llvm_func_map | ||
# MULTI-NEXT: Type: SHT_LLVM_FUNC_MAP | ||
# MULTI-NEXT: Entries: | ||
# MULTI-NEXT: - Version: 1 | ||
# MULTI-NEXT: Feature: 0x1 | ||
# MULTI-NEXT: Address: 0x2 | ||
# MULTI-NEXT: DynInstCnt: 0x3 | ||
# MULTI-NEXT: - Name: '.llvm_func_map (1)' | ||
# MULTI-NEXT: Type: SHT_LLVM_FUNC_MAP | ||
# MULTI-NEXT: Entries: | ||
# MULTI-NEXT: - Version: 1 | ||
# MULTI-NEXT: Feature: 0x1 | ||
# MULTI-NEXT: Address: 0xA | ||
# MULTI-NEXT: DynInstCnt: 0xB | ||
|
||
--- !ELF | ||
FileHeader: | ||
Class: ELFCLASS64 | ||
Data: ELFDATA2LSB | ||
Type: ET_EXEC | ||
Sections: | ||
- Name: .llvm_func_map | ||
Type: SHT_LLVM_FUNC_MAP | ||
Entries: | ||
- Version: 1 | ||
Feature: 0x1 | ||
Address: 0x2 | ||
DynInstCnt: 0x3 | ||
- Name: '.llvm_func_map (1)' | ||
Type: SHT_LLVM_FUNC_MAP | ||
Entries: | ||
- Version: 1 | ||
Feature: 0x1 | ||
Address: 0xA | ||
DynInstCnt: 0xB | ||
|
||
## Check that obj2yaml uses the "Content" tag to describe an .llvm_func_map section | ||
## when it can't extract the entries, for example, when the section is truncated. | ||
|
||
# RUN: yaml2obj --docnum=1 -DSIZE=0x8 %s -o %t4 | ||
# RUN: obj2yaml %t4 | FileCheck %s --check-prefixes=TRUNCATED,INVALID | ||
|
||
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nit: double blank line |
||
# INVALID: --- !ELF | ||
# INVALID-NEXT: FileHeader: | ||
# INVALID-NEXT: Class: ELFCLASS64 | ||
# INVALID-NEXT: Data: ELFDATA2LSB | ||
# INVALID-NEXT: Type: ET_EXEC | ||
# INVALID-NEXT: Sections: | ||
# INVALID-NEXT: - Name: .llvm_func_map | ||
# INVALID-NEXT: Type: SHT_LLVM_FUNC_MAP | ||
# BADNUM-NEXT: Content: {{([[:xdigit:]]+)}}{{$}} | ||
# TRUNCATED-NEXT: Content: '{{([[:xdigit:]]{16})}}'{{$}} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'd be tempted to explicitly check the expected 8 bytes here, to show that it's not producing garbage here. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please also update https://llvm.org/docs/Extensions.html.