Points to existing documentation for branch_weights and
function_entry_count, and adds an example for VP value profile metadata.
Details
Diff Detail
- Build Status
Buildable 7258 Build 7258: arc lint + arc unit
Event Timeline
docs/LangRef.rst | ||
---|---|---|
5210 | Does it attach to the call instruction? It can be attached to SELECT instruction which should be mentioned. | |
5220 | to record the number of times the function is called. Used with BFI information, it is also used to derive basic block profile count. | |
5231 | memcpy -- mempy like calls (including memset, etc) |
docs/LangRef.rst | ||
---|---|---|
5210 | It attached to direct call instructions in SamplePGO mode. And according to the linked older documentation I found (http://llvm.org/docs/BranchWeightMetadata.html#indirectbrinst) it attaches to indirect calls, although I wonder if that is stale since we now have VP metadata for indirect calls? Will add "select" to the list of instruction types here. | |
5220 | will update. | |
5231 | will change to "memory intrinsics such as memcpy, memmove and memset" |
docs/LangRef.rst | ||
---|---|---|
5210 | Need to document the direct call usage. indirectbr is not for indirect call, but computed gotos. So may be expand branch into conditional branch or indirect branches? Unconditional direct branch does not need this meta data. |
Thank you for acting on this quickly. Comments inline, but nothing here should prevent this landing. We can iterate on the documentation once there's *something* in place.
docs/BranchWeightMetadata.rst | ||
---|---|---|
70 ↗ | (On Diff #102602) | Please describe the semantics of the metadata separately from the usage. I have an alternate profiling source and need to know how the metadata is used, not just where you assume it comes from. |
docs/LangRef.rst | ||
5237 | It would have been more clear to represent the types as strings in the metadata. Document what's there for the moment, but might be worth changing this. Clarification: Is it legal for the sum of the pairs to not add up to the total? (It should be.) Clarification: Is it *required* that the counts be exact executio counts? Or are they solely relative weightings? (Should be the later.) |
Sure. Can you or David LGTM the patch if this looks good for now?
docs/BranchWeightMetadata.rst | ||
---|---|---|
70 ↗ | (On Diff #102602) | Right now it is only used in SamplePGO mode (used to detect hot calls since profile counts on the function entry etc may not be accurate in sampling mode). I've updated the wording to indicate this. |
docs/LangRef.rst | ||
5237 |
David may be able to give some context on why it is an int and not a string. But as a user I agree a string would be clearer.
Yes. In fact, we only profile the top N (N is configurable, default is 8), so the value counts often don't add up to the total. I have clarified this.
Relative weightings should work fine. In fact, the total may not be equal to the BB execution count in a multi-threaded profile collection environment since counter updates are non-atomic. |
Does it attach to the call instruction?
It can be attached to SELECT instruction which should be mentioned.