[PATCH] D32877: Restrict call metadata based hotness detection to Sample PGO mode

[Teresa Johnson via Phabricator via llvm-commits]

tejohnson added inline comments.


================
Comment at: include/llvm/Analysis/ProfileSummaryInfo.h:62
+                                            BlockFrequencyInfo *BFI,
+                                            ProfileSummary *Summary = nullptr);
   /// \brief Returns true if \p F has hot function entry.
----------------
eraman wrote:
> A major rationale for adding ProfileSummaryInfo as a separate analysis is to prevent Profilesummary from being directly manipulated, so I believe we shouldn't add an interface that takes ProfileSummary as a parameter.  As Dehao mentions below, you anyway get the module from the instruction and get the kind from there, so this is not necessary. My concern there is this becomes unnecessarily expensive as we get an invariant value (kind) many times. 
> A major rationale for adding ProfileSummaryInfo as a separate analysis is to prevent Profilesummary from being directly manipulated, so I believe we shouldn't add an interface that takes ProfileSummary as a parameter. 

Note that this is only passed in when getProfileCount() is called from a ProfileSummaryInfo method (it passes in the Summary object it owns), so I am not sure what the concern is about directly manipulating it?

> As Dehao mentions below, you anyway get the module from the instruction and get the kind from there, so this is not necessary. My concern there is this becomes unnecessarily expensive as we get an invariant value (kind) many times.

Exactly, that's why I don't think we should keep re-finding the profile metadata every time when it is called from the ProfileSummaryInfo.




https://reviews.llvm.org/D32877