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

[David Blaikie via Phabricator via llvm-commits]

dblaikie added a comment.

In D58406#1404169 <https://reviews.llvm.org/D58406#1404169>, @fedor.sergeev wrote:

> The need for generic way of getting textual description for IRUnit is obvious both in legacy and new pass manager.
>  However in this particular approach I dont like how "const void*" is used to erase specific type of IRUnit.
>
> In new pass manager there was a similar task of erasing the type of IRUnit for pass-instrumentation purposes.
>  And there we decided to wrap the pointer into llvm::Any.
>
> I'm not sure llvm::Any is the best approach here, but it sounds better than absolute anarchy of const void*.


Fair.

Alternatively - how would you feel about callers passing in the strings directly?

  return !BisectEnabled || checkPass(P->getPassName(), P->getDescription(U));

The Pass and Unit are only used to retrieve the Pass Name and Description (passing the Unit straight back to the Pass that just passed it in) - so why not pass in those (Pass Name and Description) instead?

I guess the way it is now, those two strings are not computed if Bisect is disabled, so there's some efficiency gain - any sense whether that's significant/important?


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

https://reviews.llvm.org/D58406