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



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to