[PATCH] D79359: OpenMPOpt Remarks Support

Joseph Huber via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue May 5 09:41:42 PDT 2020


jhuber6 marked an inline comment as done.
jhuber6 added inline comments.


================
Comment at: llvm/lib/Transforms/IPO/OpenMPOpt.cpp:612
+      return std::make_unique<OptimizationRemarkEmitter>(F);
+    };
+
----------------
jhuber6 wrote:
> jdoerfert wrote:
> > jhuber6 wrote:
> > > jdoerfert wrote:
> > > > Call getAnalysis here?
> > > I tried using the getAnalysis method but it caused it to seg-fault. I'm not exactly confident why but some other passes generate the ORE this way so I figured it was okay.
> > Oh, right. Yeah, so, the old PM CGSCC pass is bad at requiring function passes. Long story short, it doesn't work.
> > 
> > We should do what the inliner does, create an ORE whenever we need one for a function. Since we would also like hotness information in the new pass manager (which I think we should get automatically via the pass), we should not always create a new ORE but only in the old PM. That said, the ORE constructor says it can be expensive if hotness filtering is requested as block frequency is computed. Thus, in the old PM we can cache the OREs ourselves. So, we keep a map from function to ORE and in the OREGetter we lookup the ORE, if not there create it, and return it. WDYT?
> I was thinking about adding a cache previously since the function being worked on might not change often. It might be somewhat difficult to bake that functionality into the callback but I'll see what I can do in the morning.
I just noticed I could probably have used try_emplace() rather than checking and inserting manually.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79359





More information about the llvm-commits mailing list