On Mon, May 16, 2022 at 11:58 AM Rui Ueyama <rui...@gmail.com> wrote:
>
> Version handshaking is doable, but it feels like we are over-designing
> an API, given that the real providers of this plugin API are only GCC
> and LLVM and the users of the API are BFD ld, gold and mold. It is
> unlikely that we'll have dozens of more compilers or linkers in the
> near future. So, I personally prefer the following style
>
> if (!strcmp(plugin_compiler_name, "gcc") && plugin_major >= 12)
>
> than versioning various API-provided functions. It'll just work and be
> easy to understand.
>
> Besides that, even if we version GCC-provided plugin API functions, we
> still need a logic similar to the above to distinguish GCC from LLVM,
> as they behave slightly differently in various corner cases. We can't
> get rid of the heuristic version detection logic from the linker
> anyways.
>
> So, from the linker's point of view, exporting a compiler name and
> version numbers is enough.

I agree that this might be convenient enough but the different behaviors
are because of inadequate documentation of the API - that's something
we should fix.  And for this I think a plugin API version might help
as we can this way also handle your case of the need of querying
whether v2 will be used or not.

I wouldn't go to enumerate past API versions - the version handshake
hook requirement alone makes that not so useful.  Instead I'd require
everybody implementing the handshare hook implementing also all
other hooks defined at that point in time and set the version to 1.

I'd eventually keep version 2 to indicate thread safety (of a part of the API).

That said, I'm not opposed to add a "GCC 12.1" provider, maybe the
version handshake should be

int api_version (int linker, const char **identifier);

where the linker specifies the desired API version and passes an
identifier identifying itself ("mold 1.0") and it will get back the API
version the plugin intends to use plus an identifier of the plugin
("GCC 12.1").

Richard.

>
>
> On Mon, May 16, 2022 at 5:38 PM Martin Liška <mli...@suse.cz> wrote:
> >
> > On 5/16/22 11:25, Jan Hubicka via Gcc-patches wrote:
> > >>
> > >> Sure having a 'plugin was compiled from sources of the GCC N.M compiler'
> > >> is useful if bugs are discovered in old versions that you by definition 
> > >> cannot
> > >> fix but can apply workarounds to.  Note the actual compiler used might 
> > >> still
> > >> differ.  Note that still isn't clean API documentation / extension of 
> > >> the plugin
> > >> API itself.  As of thread safety we can either add a claim_file_v2_hook
> > >> or try to do broader-level versioning of the API with a new api_version
> > >> hook which could also specify that with say version 2 the plugin will
> > >> not use get_symbols_v2 but only newer, etc.  The linker would announce
> > >> the API version it likes to use and the plugin would return the
> > >> (closest) version
> > >> it can handle.  A v2 could then also specify that claim_file needs to be
> > >
> > > Yep, I think having the API version handshake is quite reasonable way to
> > > go as the _v2,_v3 symbols seems already getting bit out of hand and we
> > > essentially have 3 revisions of API to think of
> > >  (first adding LDPR_PREVAILING_DEF_IRONLY_EXP, second adding
> > >  GET_SYMBOLS_V3 and now we plan third adding thread safety and solving
> > >  the file handler problems)
> >
> > How should be design such a version handshake?
> >
> > >
> > >> thread safe, or that cleanup should allow a followup onload again with
> > >> a state identical to after dlopen, etc.
> > >>
> > >> Is there an API document besides the header itself somewhere?
> > >
> > > There is https://gcc.gnu.org/wiki/whopr/driver
> > > I wonder if this can't be moved into more official looking place since
> > > it looks like it is internal to GCC WHOPR implementation this way.
> >
> > We can likely add it here:
> > https://gcc.gnu.org/onlinedocs/gccint/LTO.html#LTO
> >
> > What do you think? I can port it.
> >
> > Martin
> >
> > >
> > > Honza
> > >>
> > >> Richard.
> >

Reply via email to