[PATCH] D58406: Fix IR/Analysis layering issue in OptBisect

[David Blaikie via Phabricator via llvm-commits]

dblaikie added a comment.

In D58406#1405264 <https://reviews.llvm.org/D58406#1405264>, @rtrieu wrote:

> Last time, this was brought up, passing strings was deemed too expensive since it would cause a string to be created every time:
>  http://lists.llvm.org/pipermail/llvm-dev/2018-March/122032.html
>
> There's a difference between getting the pass name and the IR unit name.  Pass names are hard-coded string literals which can be cheaply referred to by a StringRef.  IR units need to create a new string with the description (except Loop and Region, which don't have interesting descriptions yet.)


Fair enough - thanks for the reference/thread!

Makes sense to me (& yeah, doing this for every Function, and the complicated CGSCC description, etc - at least for Function/Module/BasicBlock passing as a Twine might suffice to delay some/enough of the computation, but CGSCC is a bit too verbose for that).

The only other option would be passing a functor (either std::function, or making "shouldRunPass" a function template) which would avoid the need for the virtuality, Any usage, etc. *shrug*

I don't feel too strongly from the available options - but hope someone with a closer connection/appreciation for these classes might have an opinion about whether adding the extra virtual function to Pass is problematic (philosophically or practically) or not.


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

https://reviews.llvm.org/D58406