[llvm] [ThinLTO] Use StringRef instead of std::string (PR #97963)
Teresa Johnson via llvm-commits
llvm-commits at lists.llvm.org
Mon Jul 8 09:02:21 PDT 2024
teresajohnson wrote:
> > > > Who is the owner of the string? Can you make sure that there is one in the bitcode reader case and also document at the structure definition who that owner is?
> > >
> > >
> > > In the bitcode reader, the owner is the [string table](https://github.com/llvm/llvm-project/blob/a5bfe20f6fd8957f870f8ffcf836d7737864e063/llvm/lib/Bitcode/Reader/BitcodeReader.cpp#L7499) of the bitcode module
Actually, it is a StringRef in the BitcodeModule as well - so not owned by that data structure either. Poking around, it appears to be a reference to the bitcode buffer. Are we guaranteed that the bitcode memory buffer. Are we guaranteed that the file stays open through the lifetime of the index? I don't believe so.
> >
> >
> > Is that guaranteed to live through the lifetime of the summary index? I didn't think so.
>
> Why it is not guaranteed?
I don't see any documentation on the BitcodeModule definition that says it must stay live past the reader, but like I said above that's not the owner either.
> Could you please give an example?
It doesn't seem to be kept after the summary is merged into the combined index in this case, for example: https://github.com/llvm/llvm-project/blob/a5bfe20f6fd8957f870f8ffcf836d7737864e063/llvm/lib/Bitcode/Reader/BitcodeReader.cpp#L8574-L8581
And if I look at the caller of that e.g. in llvm-lto.cpp, the file buffer in memory is not kept open across the lifetime of the combined index.
>
> > > In the ll parser, the owner is the index itself since this patch uses Index->saveString to save the string in the index.
https://github.com/llvm/llvm-project/pull/97963
More information about the llvm-commits
mailing list