-
Notifications
You must be signed in to change notification settings - Fork 10.8k
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
[nfc][docs]Generalize header description and ascii art for indexed profiles #83507
Conversation
minglotus-6
commented
Mar 1, 2024
- Add pointers to code for source of truth.
- Move necessary details from doc to code.
- Add pointers to code for source of truth. - Move necessary details from doc to code.
I think the doc is easier to maintain by removing boilerplates. Otherwise it's painful to update ascii art graph with templated content. Planned to submit this change before #83309. |
llvm/docs/InstrProfileFormat.rst
Outdated
| | +-----------------------+ O | ||
| +->| Binary Ids | A | ||
| Byte Offset | A | ||
+------ | of section 2 | D |
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.
why starting the number from 2?
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.
Profile summary is regarded as the first payload section and it doesn't need an offset.
I revised the graph to use section A B ... Z, and added a note to mention profile summary.
llvm/docs/InstrProfileFormat.rst
Outdated
``HashType`` | ||
The hashing scheme for on-disk hash table keys. Only MD5 hashing is used as of | ||
writing. | ||
The `Header struct`_ should be the source of truth. Field names and comments |
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.
Probably document the sections are A, B, C, and D, and their order is defined in Header struct.
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.
Incorporate the section names in the graph above and reworded the paragraph slightly.
For indexed profiles, the byte offset is relative to the start of profile memory buffer. Thereby the field order in the struct definition doesn't need to be the same as the order of payloads in the profile (e.g., if we need it sometime in the future).
Bump. |