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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
