Re: [PATCH] Add .gnu.lto_.meta section.

[Richard Biener <richard.guenther@gmail.com>]

On Tue, Jun 25, 2019 at 10:14 AM Martin Liška <mliska@suse.cz> wrote:
>
> On 6/24/19 8:05 PM, Richard Biener wrote:
> > On Mon, Jun 24, 2019 at 3:31 PM Martin Liška <mliska@suse.cz> wrote:
> >>
> >> On 6/24/19 2:44 PM, Richard Biener wrote:
> >>> On Mon, Jun 24, 2019 at 2:12 PM Martin Liška <mliska@suse.cz> wrote:
> >>>>
> >>>> On 6/24/19 2:02 PM, Richard Biener wrote:
> >>>>> On Fri, Jun 21, 2019 at 4:01 PM Martin Liška <mliska@suse.cz> wrote:
> >>>>>>
> >>>>>> On 6/21/19 2:57 PM, Jan Hubicka wrote:
> >>>>>>> This looks like good step (and please stream it in host independent
> >>>>>>> way). I suppose all these issues can be done one-by-one.
> >>>>>>
> >>>>>> So there's a working patch for that. However one will see following errors
> >>>>>> when using an older compiler or older LTO bytecode:
> >>>>>>
> >>>>>> $ gcc main9.o -flto
> >>>>>> lto1: fatal error: bytecode stream in file ‘main9.o’ generated with LTO version -25480.4493 instead of the expected 9.0
> >>>>>>
> >>>>>> $ gcc main.o
> >>>>>> lto1: internal compiler error: compressed stream: data error
> >>>>>
> >>>>> This is because of your change to bitfields or because with the old
> >>>>> scheme the header with the
> >>>>> version is compressed (is it?).
> >>>>
> >>>> Because currently also the header is compressed.
> >>>
> >>> That was it, yeah :/  Stupid decisions in the past.
> >>>
> >>> I guess we have to bite the bullet and do this kind of incompatible
> >>> change, accepting
> >>> the odd error message above.
> >>>
> >>>>> I'd simply avoid any layout changes
> >>>>> in the version check range.
> >>>>
> >>>> Well, then we have to find out how to distinguish between compression algorithms.
> >>>>
> >>>>>
> >>>>>> To be honest, I would prefer the new .gnu.lto_.meta section.
> >>>>>> Richi why is that so ugly?
> >>>>>
> >>>>> Because it's a change in the wrong direction and doesn't solve the
> >>>>> issue we already
> >>>>> have (cannot determine if a section is compressed or not).
> >>>>
> >>>> That's not true, the .gnu.lto_.meta section will be always uncompressed and we can
> >>>> also backport changes to older compiler that can read it and print a proper error
> >>>> message about LTO bytecode version mismatch.
> >>>
> >>> We can always backport changes, yes, but I don't see why we have to.
> >>
> >> I'm fine with the backward compatibility break. But we should also consider lto-plugin.c
> >> that is parsing following 2 sections:
> >>
> >>     91  #define LTO_SECTION_PREFIX      ".gnu.lto_.symtab"
> >>     92  #define LTO_SECTION_PREFIX_LEN  (sizeof (LTO_SECTION_PREFIX) - 1)
> >>     93  #define OFFLOAD_SECTION         ".gnu.offload_lto_.opts"
> >>     94  #define OFFLOAD_SECTION_LEN     (sizeof (OFFLOAD_SECTION) - 1)
> >
> > Yeah, I know.  And BFD and gold hard-coded those __gnu_lto_{v1,slim} symbols...
>
> Yep, they do, 'nm' is also using that.
>
> >
> >>>
> >>>>> ELF section overhead
> >>>>> is quite big if you have lots of small functions.
> >>>>
> >>>> My patch is actually shrinking space as I'm suggesting to add _one_ extra ELF section
> >>>> and remove the section header from all other LTO sections. That will save space
> >>>> for all function sections.
> >>>
> >>> But we want the header there to at least say if the section is
> >>> compressed or not.
> >>> The fact that we have so many ELF section means we have the redundant version
> >>> info everywhere.
> >>>
> >>> We should have a single .gnu.lto_ section (and also get rid of those
> >>> __gnu_lto_v1 and __gnu_lto_slim COMMON symbols - checking for
> >>> existence of a symbol is more expensive compared to existence
> >>> of a section).
> >>
> >> I like removal of the 2 aforementioned sections. To be honest I would recommend to
> >> add a new .gnu.lto_.meta section.
> >
> > Why .meta?  Why not just .gnu.lto_?
>
> Works for me.
>
> >
> >> We can use it instead of __gnu_lto_v1 and we can
> >> have a flag there instead of __gnu_lto_slim. As a second step, I'm willing to concatenate all
> >>
> >>   LTO_section_function_body,
> >>   LTO_section_static_initializer
> >>
> >> sections into a single one. That will require an index that will have to be created. I can discuss
> >> that with Honza as he suggested using something smarter than function names.
> >
> > I think the index belongs to symtab?
> >
> > Let's properly do it if we want to change it.  Removing of
> > __gnu_lto_v1/slim is going to be
> > the most intrusive change btw. and orthogonal to the section changes.
>
> I'm fine with a proper change. So do I understand that correctly that:
> - we'll come up with .gnu.lto_ section that will be used by bfd, gold and nm
>   to detect LTO objects
> - for some time, we'll keep __gnu_lto_v1 and __gnu_lto_slim for backward
>   compatibility with older binutils tool
> - in couple of year, the legacy support will be removed

Yep.

Richard.

> ?
>
> Martin
>
> >
> > Richard.
> >
> >>
> >> Thoughts?
> >> Martin
> >>
> >>>
> >>> Richard.
> >>>
> >>>> Martin
> >>>>
> >>>>>
> >>>>> Richard.
> >>>>>
> >>>>>>
> >>>>>> Martin
> >>>>
> >>
>