[PATCH] D41604: Add a pass to generate synthetic function entry counts.

[David Li via Phabricator via llvm-commits]

davidxl added inline comments.


================
Comment at: include/llvm/Analysis/SyntheticCountsUtils.h:27
+using Scaled64 = ScaledNumber<uint64_t>;
+void propagateSyntheticCounts(
+    const CallGraph &CG, function_ref<Scaled64(CallSite CS)> GetCallSiteWeight,
----------------
add documentation comments here.


================
Comment at: include/llvm/IR/Function.h:256
+  /// will execute.
+  Optional<uint64_t> getSyntheticEntryCount() const;
+
----------------
Is it better to let getEntryCount to take an optional flag to indicate 'is_synthetic'? The implementation of these two are mostly the same.


================
Comment at: lib/Transforms/IPO/SyntheticCountsPropagation.cpp:48
+/// Initial synthetic count assigned to functions.
+static cl::opt<int>
+    InitialSyntheticCount("initial-synthetic-count", cl::Hidden, cl::init(10),
----------------
We should probably differentiate functions with cold attributes and set them with lower initial count.


================
Comment at: lib/Transforms/IPO/SyntheticCountsPropagation.cpp:70
+      InitialCount = InlineSyntheticCount;
+    } else if (F.hasLocalLinkage()) {
+      // Local functions without inline hints get counts only through
----------------
This should be restricted to local functions not address escaped. If the local function is only called indirectly, setting its initial count to zero can make it look too cold. For address exposed ones, the initial count should be handled similarly to global functions.


https://reviews.llvm.org/D41604