[PATCH] D73817: Add PassManagerImpl.h to hide implementation details

[David Blaikie via Phabricator via llvm-commits]

dblaikie added a comment.

In D73817#1855867 <https://reviews.llvm.org/D73817#1855867>, @rnk wrote:

> @aprantl, thanks, I fixed it in a follow-up module.map change: rGf8c4d70d1138 <https://reviews.llvm.org/rGf8c4d70d11388ddbf3ccc63ca4ea35d09a987d41>.
>
> In D73817#1855806 <https://reviews.llvm.org/D73817#1855806>, @dblaikie wrote:
>
> > Any idea why is this class being instantiated/spending a lot of time on that instantiation if it's the subject of an explicit instantiation decl/def?
>
>
> As for why it's instantiated, most TUs instantiate AnalysisManager::getResult. I see 83 files with call sites in lib/Transforms. So, it is actually commonly required functionality. I haven't nailed down if it gets instantiated accidentally just from including PassManager.h. I think the answer is no, but either way this is an improvement.


But there's an explicit instantiation declaration, right? Which should prevent any implicit instantiation.

> As for why it's expensive, there are two DenseMaps in AnalysisManager, and this function inserts into both maps. In -ftime-trace, those instantiations happen to be attributed to getResultImpl. It's possible that some of those DenseMapPair & std::pair instantiations happen later anyway after this change, but you can see the memory usage reduction is real.

My understanding is that the explicit instantiation declaration should actually make it invalid for the C++ compiler to instantiate the function at all, except where it sees the explicit instantiation definition.

For instance this example:

template<typename T>
void f1() {

  typename T::u v;

}
extern template void f1<int>();
int main() {

  f1<int>();

}

Successfully compiles because the compiler doesn't instantiate the contents of f1<int> due to the presence of the explicit instantiation declaration. If you add the "inline" keyword to the f1 declaration, then the compiler is allowed to instantiate f1<int> in this TU and it does reject it due to "T::u" being invalid for T=int.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73817