[PATCH] D34218: [Doc] Document prof metadata in LangRef

[Teresa Johnson via Phabricator via llvm-commits]

tejohnson added a comment.

In https://reviews.llvm.org/D34218#780743, @reames wrote:

> 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.


Sure. Can you or David LGTM the patch if this looks good for now?



================
Comment at: docs/BranchWeightMetadata.rst:70
+
+In SamplePGO mode, direct calls only have branch weight metadata,
+containing the execution count of the call.
----------------
reames wrote:
> 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.
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.


================
Comment at: docs/LangRef.rst:5237
+is executed, followed by uint64_t value and execution count pairs.
+The value profiling kind is 0 for indirect call targets and 1 for memory
+operations. For indirect call targets, each profile value is a hash
----------------
reames wrote:
> 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.)
> 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.

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.

> Clarification: Is it legal for the sum of the pairs to not add up to the total? (It should be.)

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.

> Clarification: Is it *required* that the counts be exact execution counts? Or are they solely relative weightings (Should be the later.)

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. 


https://reviews.llvm.org/D34218