[PATCH] D128854: [MemProf] Add memprof metadata related analysis utilities

[Snehasish Kumar via Phabricator via llvm-commits]

snehasish added a comment.

lgtm, will go through D128142 <https://reviews.llvm.org/D128142> soon. Thanks!



================
Comment at: llvm/unittests/Analysis/MemoryProfileInfoTest.cpp:299
+// context required to identify the allocation type.
+TEST_F(MemoryProfileInfoTest, ReTrimMIBContext) {
+  LLVMContext C;
----------------
tejohnson wrote:
> snehasish wrote:
> > I think this test is redundant since at this point we have already tested
> > * the ability to trim contexts in `TrimmedMIBContext`
> > * the ability to extract the metadata node and call the underlying addCallStack() correctly in `SimplifyMIBToAttribute`.
> > 
> > Up to you if you want to keep it but I'm slightly in favour of removing this test if it doesn't cover any new logic.
> I wanted to have a test that put these together since is functionality that will be used to update after inlining. I agree the components are tested separately already, but I kind of like having this for completeness. If you prefer though I can remove it.
Keeping as is for completeness sounds reasonable to me.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128854