[PATCH] D12781: PGO IR-level instrumentation infrastructure

[Sean Silva via llvm-commits]

silvas added a comment.

A couple more comments I spotted when going back through the patch again.


================
Comment at: include/llvm/ProfileData/InstrProfWriter.h:44-45
@@ -43,3 +43,4 @@
                                     uint64_t FunctionHash,
-                                    ArrayRef<uint64_t> Counters);
+                                    ArrayRef<uint64_t> Counters,
+                                    bool IsLateInstr = false);
   /// Write the profile to \c OS
----------------
Why do we need to modify InstrProfWriter.{h,cpp}? That seems like a layering violation.

================
Comment at: lib/Transforms/Instrumentation/PGOLateInstr.cpp:270-278
@@ +269,11 @@
+
+PGOLateInstrumentationFunc::~PGOLateInstrumentationFunc() {
+  // Free Edge in AllEdges.
+  for (auto &EI : AllEdges)
+    delete EI;
+
+  // Free BBInfo in BBInfos.
+  for (auto &BBGI : BBInfos)
+    delete BBGI.second;
+}
+
----------------
No naked new/delete. Use RAII (e.g. you can probably use BumpPtrAllocator to allocate these).


http://reviews.llvm.org/D12781