On Tue, Aug 20, 2024 at 2:03 AM Richard Biener
<richard.guent...@gmail.com> wrote:
>
> On Wed, Aug 14, 2024 at 3:15 PM H.J. Lu <hjl.to...@gmail.com> wrote:
> >
> > The new hook allows the linker plugin to distinguish calls to
> > claim_file_handler that know the object is being used by the linker
> > (from ldmain.c:add_archive_element), from calls that don't know it's
> > being used by the linker (from elf_link_is_defined_archive_symbol); in
> > the latter case, the plugin should avoid including the unused LTO archive
> > members in linker output. To get the proper support for archives with
> > LTO common symbols, the linker fix for
> >
> > https://sourceware.org/bugzilla/show_bug.cgi?id=32083
> >
> > is required.
> >
> > PR lto/116361
> > * lto-plugin.c (claim_file_handler_v2): Include the LTO object
> > only if it is known to be used for link output.
> >
> > Signed-off-by: H.J. Lu <hjl.to...@gmail.com>
> > ---
> > lto-plugin/lto-plugin.c | 20 ++++++++++++--------
> > 1 file changed, 12 insertions(+), 8 deletions(-)
> >
> > diff --git a/lto-plugin/lto-plugin.c b/lto-plugin/lto-plugin.c
> > index 152648338b9..2d2bfa60d42 100644
> > --- a/lto-plugin/lto-plugin.c
> > +++ b/lto-plugin/lto-plugin.c
> > @@ -1286,13 +1286,17 @@ claim_file_handler_v2 (const struct
> > ld_plugin_input_file *file, int *claimed,
> > lto_file.symtab.syms);
> > check (status == LDPS_OK, LDPL_FATAL, "could not add symbols");
>
> We are still doing add_symbols, shouldn't what we do depend on what
> that does? The
If status != LDPS_OK, the plugin will abort because of LDPL_FATAL.
> function comment says
>
> If KNOWN_USED, the object is known by the linker
> to be used, or an older API version is in use that does not provide that
> information; otherwise, the linker is only determining whether this is
> a plugin object and it should not be registered as having offload data if
> not claimed by the plugin.
>
> where do you check "if not claimed by the plugin"? I think this at least
> needs
> clarification with the change.
See my reply below.
> > - LOCK_SECTION;
> > - num_claimed_files++;
> > - claimed_files =
> > - xrealloc (claimed_files,
> > - num_claimed_files * sizeof (struct plugin_file_info));
> > - claimed_files[num_claimed_files - 1] = lto_file;
> > - UNLOCK_SECTION;
> > + /* Include it only if it is known to be used for link output. */
> > + if (known_used)
> > + {
> > + LOCK_SECTION;
> > + num_claimed_files++;
> > + claimed_files =
> > + xrealloc (claimed_files,
> > + num_claimed_files * sizeof (struct plugin_file_info));
> > + claimed_files[num_claimed_files - 1] = lto_file;
> > + UNLOCK_SECTION;
> > + }
> >
> > *claimed = 1;
> > }
> > @@ -1313,7 +1317,7 @@ claim_file_handler_v2 (const struct
> > ld_plugin_input_file *file, int *claimed,
> > if (*claimed && !obj.offload && offload_files_last_lto == NULL)
> > offload_files_last_lto = offload_files_last;
> >
> > - if (obj.offload && (known_used || obj.found > 0))
> > + if (obj.offload && known_used && obj.found > 0)
The offload data is included when it is claimed by the plugin
even if known_used is 0. It looks quite odd to me. Since
can't test it and it isn't needed for PR lto/116361, I dropped
this change in the v2 patch:
https://gcc.gnu.org/pipermail/gcc-patches/2024-August/660539.html
If you agree that this change is correct, I can include it and update
comments.
> > {
> > /* Add file to the list. The order must be exactly the same as the
> > final
> > order after recompilation and linking, otherwise host and target
> > tables
> > --
> > 2.46.0
> >
--
H.J.
