[PATCH] D53890: [LTO] Record LTOUnit flag in index and validate during LTO link

[Peter Collingbourne via Phabricator via llvm-commits]

pcc added a comment.

In https://reviews.llvm.org/D53890#1297566, @tejohnson wrote:

> In https://reviews.llvm.org/D53890#1297383, @pcc wrote:
>
> > In https://reviews.llvm.org/D53890#1297258, @tejohnson wrote:
> >
> > > In https://reviews.llvm.org/D53890#1293818, @pcc wrote:
> > >
> > > > > without LTOUnit (for purely summary-based class hierarchy analyses).
> > > >
> > > > We might want a different name for the flag then, because a translation unit that is subject to purely summary-based class hierarchy analysis is still part of the LTO unit, it just participates in fewer optimizations.
> > >
> > >
> > > I think I am confused then about the terminology. My understanding was that the LTO unit was the part that would be linked with regular LTO. Does a linkage unit's LTO unit include both the regular and thin LTO modules?
> >
> >
> > The LTO unit is a user-facing concept, and is (currently) defined as follows in LTOVisibility.rst:
> >
> > > a linkage ​unit's *LTO unit* is the subset of the linkage unit that is linked together ​using link-time optimization
> >
> > The splitting of a translation unit into regular and thin LTO modules is an implementation detail that allows the LTO unit features described in that document to work. So I suppose that it would be accurate to say that both the regular and thin LTO modules are part of the LTO unit, but that's just a consequence of how things are implemented. Hope that helps.
>
>
> Ok, I read that, but it wasn't clear to me what it meant in the context of ThinLTO and regular LTO split modules.
>
> > 
> > 
> >> If so, what name would you suggest?
> > 
> > On second thoughts, it does seem reasonable for the flag to be called LTOUnit. This reflects that the translation unit is part of the LTO unit.
> > 
> > To go over the cases, we will have:
> > 
> > 1. Regular LTO, part of the LTO unit
> > 2. ThinLTO, "traditional" WPD, part of the LTO unit
> > 3. ThinLTO, not part of the LTO unit
> > 
> >   and eventually:
> > 4. ThinLTO, summary-based WPD, part of the LTO unit
>
> I am confused on the difference between 3 and 4. What makes that augmented summary that enables the WPD (i.e. 4) imply that it is now part of the LTO unit, whereas currently with ThinLTO -fno-lto-unit which implies other whole-program optimizations (i.e. 3) it is not? Reading through https://clang.llvm.org/docs/LTOVisibility.html again, I guess it is because with WPD we need to talk about LTO visibility, and that can only be done in the context of an LTO unit property. So simply doing class hierarchy optimizations such as WPD we require that the ThinLTO module be defined as being part of the LTO unit?


Yes. Basically when you're adding summary-based WPD you're adding another way in which the TU can be part of the LTO unit.

> It sounds to  me like changing the default of the -flto-unit flag might be wrong, and what we really want is a different flag that controls the actual splitting directly?

That sounds like it would be simpler. We could present a choice between "use summary-based WPD" and "use traditional WPD", where the default is to use summary-based WPD, and record that in a flag. Then we wouldn't need to change the definition of an LTO unit, and I think we would only need to track whether that flag is inconsistent.


Repository:
  rL LLVM

https://reviews.llvm.org/D53890