[PATCH] D47858: [New PM] Introducing PassInstrumentation framework

Philip Pfaffe via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Aug 21 04:26:35 PDT 2018


philip.pfaffe added a comment.

Thanks for moving this forward!

Because of vacation I've only had time for a quick glance.  Two somewhat higher-level comments:

- I don't think `PassBuilder::crossRegisterProxies` is the right place to register these analyses. If I set up e.g. just a function pipeline with no proxying whatsoever, I still might want to use instrumentation. So `register*Analyses` seems like a better place to put it.
- The void*/IRUnit handling makes me a bit uncomfortable. Fundamentally, do we actually want instrumentations to have access to the actual IRUnits instead of opaque identifiers/void*? How do we prevent them from messing with it in any way? We can certainly document this and hope for the best, but should this be more radical?

If we do keep the IRUnit handling, however, you can implement it much nicer in terms of llvm::any, and thus avoid the typeid hackery you're forced to do here.


Repository:
  rL LLVM

https://reviews.llvm.org/D47858





More information about the llvm-commits mailing list