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); + } ---------------- tejohnson wrote: > 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. D154872. I will rebase this patch once that is committed. 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
