[PATCH] D154856: [MemProf] Use new option/pass for profile feedback and matching

[Teresa Johnson via Phabricator via llvm-commits]

tejohnson added inline comments.


================
Comment at: llvm/lib/Transforms/Instrumentation/MemProfiler.cpp:912
+    const TargetLibraryInfo &TLI = FAM.getResult<TargetLibraryAnalysis>(F);
+    readMemprof(M, F, MemProfReader.get(), TLI);
+  }
----------------
snehasish wrote:
> I think we can we split this patch into two to make the changes clearer. Consider the following -- 
> 
> First move the readMemprof and its callees to MemProfiler.cpp and call it from the original code. Also in this step consider using Error as the return type of readMemprof, right now the bool returned is ignored. Using Error doesn't need to be fatal, we can handle it and emit a warning but it would enforce the check I think.
> 
> Second, add the pass to MemProfiler and all the related options/plumbing.
> 
> What do you think? 
Ok let me try that. I think actually readMemprof should have a void return. The only time we are returning false the error has already been handled within readMemprof itself.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D154856