[PATCH] D41883: Add a ProfileCount class to represent entry counts.

[David Li via Phabricator via llvm-commits]

davidxl added inline comments.


================
Comment at: include/llvm/IR/Function.h:237
 
+  enum ProfileCountType { PCT_Invalid, PCT_Real, PCT_Synthetic };
+
----------------
Perhaps also split sample count and instrumentation count?


================
Comment at: include/llvm/IR/Function.h:246
+    ProfileCountType PCT;
+    static ProfileCount Invalid;
+
----------------
Why is there a need for the static member?


================
Comment at: include/llvm/IR/Function.h:254
+    uint64_t getCount() const { return Count; }
+    uint64_t getValue() const { return Count; }
+    ProfileCountType getType() const { return PCT; }
----------------
Do we need to interfaces for the same thing? I suggest removing one.


================
Comment at: include/llvm/IR/Function.h:255
+    uint64_t getValue() const { return Count; }
+    ProfileCountType getType() const { return PCT; }
+    bool isSynthetic() const { return PCT == PCT_Synthetic; }
----------------
Is this interface useful?


================
Comment at: include/llvm/IR/Function.h:260
+    // Update the count retaining the same profile count type.
+    ProfileCount &setCount(uint64_t C) {
+      Count = C;
----------------
Perhaps also add a parameter for the type?

This can avoid dropping count silently when the type mismatches.


https://reviews.llvm.org/D41883