[PATCH] D128141: [MemProf] Basic metadata support and verification

[Snehasish Kumar via Phabricator via llvm-commits]

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

lgtm with a couple of minor comments.



================
Comment at: llvm/lib/IR/Verifier.cpp:4536
+
+void Verifier::visitCallsiteMetadata(Instruction &I, MDNode *MD) {
+  visitCallStackMetadata(MD);
----------------
Instruction& I is unused. 

Perhaps add a check here that I is a CallInst? If you do then also add a test for it.


================
Comment at: llvm/lib/IR/Verifier.cpp:4537
+void Verifier::visitCallsiteMetadata(Instruction &I, MDNode *MD) {
+  visitCallStackMetadata(MD);
+}
----------------
Perhaps add a comment here about whats being verified. The transition from callsite verification to callstack may be hard to follow. How about something like --

// Here we verify the callstack profile annotated by memprof. This callsite is a part of a profiled allocation callstack.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D128141/new/

https://reviews.llvm.org/D128141