[llvm] [nfc][InstrFDO]Encapsulate header writes in a class member function (PR #90142)
Kazu Hirata via llvm-commits
llvm-commits at lists.llvm.org
Thu May 16 19:07:10 PDT 2024
================
@@ -763,16 +771,25 @@ Error InstrProfWriter::writeImpl(ProfOStream &OS) {
}
InfoObj->CSSummaryBuilder = nullptr;
+ const size_t MemProfOffset =
+ BackPatchStartOffset + sizeof(IndexedInstrProf::Header::HashOffset);
+ const size_t BinaryIdOffset =
+ MemProfOffset + sizeof(IndexedInstrProf::Header::MemProfOffset);
+ const size_t TemporalProfTracesOffset =
+ BinaryIdOffset + sizeof(IndexedInstrProf::Header::BinaryIdOffset);
+ const size_t VTableNamesOffset =
+ TemporalProfTracesOffset +
+ sizeof(IndexedInstrProf::Header::TemporalProfTracesOffset);
if (!WritePrevVersion) {
// Now do the final patch:
PatchItem PatchItems[] = {
// Patch the Header.HashOffset field.
- {HashTableStartFieldOffset, &HashTableStart, 1},
+ {BackPatchStartOffset, &HashTableStart, 1},
----------------
kazutakahirata wrote:
May I suggest grouping these three variables?
```
uint64_t HeaderUpdates[] = {
HashTableStart,
MemProfSectionStart,
BinaryIdSectionStart
};
PatchItem PatchItems[] = {
{BackPatchStartOffset, HeaderUpdate, std::size(HeaderUpdates)},
...
```
I am suggesting this because:
- The essence of the header format is that these fields are written out one after another. We don't really care about the offsets of these individual fields.
- Grouping them reduces the maintenance burden. Adding a new section just involves adding another element to the `HeaderUpdates` array above.
- The `HeaderUpdates` array concisely describes the header format, at least the portion that needs patching.
Now, if you choose not to group them, I would write:
```
const size_t MemProfOffset = BackPatchStartOffset + sizeof(uint64_t);
```
because the size of each header field on disk comes from:
```
void write(uint64_t V) { LE.write<uint64_t>(V); }
```
not the type of `IndexedInstrProf::Header::MemProfOffset`, etc.
https://github.com/llvm/llvm-project/pull/90142
More information about the llvm-commits
mailing list