[PATCH] D41297: [ThinLTO] Implement summary visualizer

Teresa Johnson via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri Jan 12 11:18:31 PST 2018


tejohnson added a comment.

In https://reviews.llvm.org/D41297#974946, @mehdi_amini wrote:

> In https://reviews.llvm.org/D41297#974754, @evgeny777 wrote:
>
> > > At which point of the API is this guarantee?
> >
> >
> >
> > 1. `ThinLTO.ModuleMap` stores `BitcodeModule` objects and uses them during entire thin link phase (until backend threads are launched)
> > 2. BitcodeModule contains pointers to BC data, so this data should be actual for module to be parsed)
> > 3. Module summary is exported to .dot in CombinedIndexHook which is invoked in the beginning of thin link phase
>
>
> None of these seems like API guarantee to me, it sounds like "this is what actually happens in practice today in this particular workflow".


It seems it must be a guarantee of the LTO API, since ThinLTO.ModuleMap holds the BitcodeModule objects and accesses their buffers through and after the thin link.

> Can I write a tool that is using the ModuleSummaryIndexBitcodeReader (or the other entry point) to create the index and release the memory buffer that contains the bitcode?

Presumably. The CombinedIndexHook that invokes this new dot dumper is only accessed via the LTO API here. But I suppose we could get in trouble if it was invoked on the index via another tool in the future. Not sure if there is a way to prevent other than documenting this StringRef clearly.



================
Comment at: include/llvm/IR/ModuleSummaryIndex.h:71
 
 struct GlobalValueSummaryInfo {
   /// The GlobalValue corresponding to this summary. This is only used in
----------------
mehdi_amini wrote:
> Are we size-sensitive here @tejohnson ?
> Since the GV field is only used in per-module and and name only in combined summary, we could have some sort of union (variant if available).
Yeah that is a concern. I like the idea of unioning with the GV.


https://reviews.llvm.org/D41297





More information about the llvm-commits mailing list