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

[Fedor Sergeev via Phabricator via llvm-commits]

fedor.sergeev added a comment.

In https://reviews.llvm.org/D47858#1207381, @philip.pfaffe wrote:

> 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.


Yes, that was definitely more of a quick hack than a proper solution.

> - 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?

In some cases IRUnits are not necessary at all, knowing the pass ID is enough (e.g. time-passes)
For other cases we need access to all the details of IRUnit (e.g. -print-before).

> 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.

I'll take a look at llvm::Any. One of the reasons I went through "typeid hackery" way was a potential ability to register callbacks for a particular sets of "IRUnit ID"s only,
but now I dont find it very compelling and llvm::Any interface appears to be a right way to go.


Repository:
  rL LLVM

https://reviews.llvm.org/D47858