[PATCH] D17624: Metadata support for profile summary

[Easwaran Raman via llvm-commits]

eraman marked 3 inline comments as done.

================
Comment at: include/llvm/ProfileData/ProfileCommon.h:59
@@ +58,3 @@
+public:
+  enum ProfileSummaryKind { PSK_Instr, PSK_Sample };
+
----------------
dnovillo wrote:
> Nit.  Do we really need the prefix 'ProfileSummary'?  This enum already needs to be referred as 'ProfileSummary::ProfileSummaryKind'.  I think 'ProfileSummary::Kind' reads a bit better.
I agree with that sentiment. I was following the example in Metadata which has an enum named MetadataKind, but I also see some other classes with just the name Kind. So I've renamed it to Kind.

================
Comment at: lib/ProfileData/ProfileSummary.cpp:255
@@ +254,3 @@
+static bool getSummaryFromMD(MDTuple *MD,
+                             std::vector<ProfileSummaryEntry> &Summary) {
+  if (!MD || MD->getNumOperands() != 2)
----------------
vsk wrote:
> `std::vector<ProfileSummaryEntry>` shows up enough to warrant having a typedef for it. Maybe that could be a follow-up?
In practice, this is going to be of fixed size (the same as DefaultCutoffs). Is SmallVector a better choice instead of std::vector ?


http://reviews.llvm.org/D17624