[PATCH] D17624: Metadata support for profile summary

[Diego Novillo via llvm-commits]

dnovillo accepted this revision.
dnovillo added a comment.
This revision is now accepted and ready to land.

Looks fine to me, though I'm not an expert on MD.


================
Comment at: include/llvm/ProfileData/ProfileCommon.h:59
@@ +58,3 @@
+public:
+  enum ProfileSummaryKind { PSK_Instr, PSK_Sample };
+
----------------
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.

================
Comment at: lib/ProfileData/ProfileSummary.cpp:150
@@ +149,3 @@
+
+// This returns a MDTuple representing the detiled summary. The tuple has two
+// elements: a string "DetailedSummary" and a MDTuple representing the value
----------------
s/a MDTuple/an MDTuple/
s/detiled/detailed/

================
Comment at: lib/ProfileData/ProfileSummary.cpp:151
@@ +150,3 @@
+// This returns a MDTuple representing the detiled summary. The tuple has two
+// elements: a string "DetailedSummary" and a MDTuple representing the value
+// of the detailed summary. Each element of this tuple is again an MDTuple whose
----------------
s/a MDTuple/an MDTuple/.  This happens in a couple other places.

================
Comment at: lib/ProfileData/ProfileSummary.cpp:318
@@ +317,3 @@
+
+// Parse an MDTuple representing an SampleProfileSummary object.
+static ProfileSummary *getSampleProfileSummaryFromMD(MDTuple *Tuple) {
----------------
s/an/a/


http://reviews.llvm.org/D17624