[PATCH] D34156: [LTO] Add -femit-summary-index flag to emit summary for regular LTO

[Peter Collingbourne via Phabricator via cfe-commits]

pcc added a comment.

In https://reviews.llvm.org/D34156#780714, @mehdi_amini wrote:

> In https://reviews.llvm.org/D34156#780570, @tobiasvk wrote:
>
> > - Change patch to always emit a module summary for regular LTO
> >
> >   I don't see any real downside of having a summary given the marginal time and space overhead it takes to construct and save it.
>
>
> I think some code/logic needs to be changed in LLVM first:
>
>   bool LTOModule::isThinLTO() {
>     // Right now the detection is only based on the summary presence. We may want
>     // to add a dedicated flag at some point.
>


Thanks for pointing that out. Right now that function will ignore full LTO summaries because they are represented using a different record number, but once https://reviews.llvm.org/D33922 lands the `hasGlobalValueSummary` function will return true if the module contains a full LTO summary. I will update that function as part of https://reviews.llvm.org/D33922 to check for presence of a ThinLTO summary.


https://reviews.llvm.org/D34156