[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#1221946, @philip.pfaffe wrote:

> Reconsidering passing the actual IRUnit to the callbacks, I still think that's not a good idea. Downstream users _will_ eventually use this to hook into the pipeline and mess with the IR somehow. So instead this should be opaque to clients.


I dont fully get your concerns.
By analogy - downstream users can use their own Analyses to mess with IR. But we do not have any kind of protection against that.
Does it worry you the same way?

> What operations do clients ever have to perform on an IRUnit? The number should be tiny I believe.
>  I'd love to see something like a generic IRUnit wrapper, that implements these operations plus some opaque IRUnit id. Currently that'd just be print.

Printing is the first user, yes.
Any kind of inspection for debug purposes (like putting  IR into database).

> Passing the wrapper to the callback then is safe, and it allows clients to both identify the ir object as well as do what we allow them to do with the object.

One of the problems with generic IRUnit wrapper is that it is not easily extensible.
A prime idea of pass instrumentation is to provide an easily extensible pass-pipeline observability tool.
I would not like to harm extensibility of this tool for the sake of protecting users against foot-shooting themselves.


Repository:
  rL LLVM

https://reviews.llvm.org/D47858