> gcc/ChangeLog:
> 
> 2020-03-12  Martin Liska  <mli...@suse.cz>
> 
>       * lto-section-in.c: Add extension_symtab.
>       * lto-streamer-out.c (write_symbol_extension_info):
>       New.
>       (produce_symtab_extension): New.
>       (produce_asm_for_decls): Stream also produce_symtab_extension.
>       * lto-streamer.h (enum lto_section_type): New section.
> 
> include/ChangeLog:
> 
> 2020-03-12  Martin Liska  <mli...@suse.cz>
> 
>       * lto-symtab.h (enum gcc_plugin_symbol_type): New.
>       (enum gcc_plugin_symbol_section_flags): Likewise.
> 
> lto-plugin/ChangeLog:
> 
> 2020-03-12  Martin Liska  <mli...@suse.cz>
> 
>       * lto-plugin.c (LTO_SECTION_PREFIX): Rename to
>       ...
>       (LTO_SYMTAB_PREFIX): ... this.
>       (LTO_SECTION_PREFIX_LEN): Rename to ...
>       (LTO_SYMTAB_PREFIX_LEN): ... this.
>       (LTO_SYMTAB_EXT_PREFIX): New.
>       (LTO_SYMTAB_EXT_PREFIX_LEN): New.
>       (LTO_LTO_PREFIX): New.
>       (LTO_LTO_PREFIX_LEN): New.
>       (parse_table_entry): Fill up unused to zero.
>       (parse_table_entry_extension): New.
>       (parse_symtab_extension): New.
>       (finish_conflict_resolution): Change type
>       for resolution.
>       (clear_new_symtab_flags): New.
>       (write_resolution): Support new get_symbols_v4.
>       (process_symtab): Use new macro name.
>       (process_symtab_extension): New.
>       (claim_file_handler): Parse also process_symtab_extension.
>       (onload): Call new add_symbols_v2.
Hi,
thanks for updating the patch.  Two minor nits
> ---
>  gcc/lto-section-in.c    |   3 +-
>  gcc/lto-streamer-out.c  |  64 +++++++++++++++-
>  gcc/lto-streamer.h      |   1 +
>  include/lto-symtab.h    |  12 +++
>  lto-plugin/lto-plugin.c | 165 +++++++++++++++++++++++++++++++++++-----
>  5 files changed, 226 insertions(+), 19 deletions(-)
> 
> diff --git a/gcc/lto-section-in.c b/gcc/lto-section-in.c
> index c17dd69dbdd..7bf59c513fc 100644
> --- a/gcc/lto-section-in.c
> +++ b/gcc/lto-section-in.c
> @@ -54,7 +54,8 @@ const char *lto_section_name[LTO_N_SECTION_TYPES] =
>    "mode_table",
>    "hsa",
>    "lto",
> -  "ipa_sra"
> +  "ipa_sra",
> +  "extension_symtab"
I would call it symtab_ext so alphabetically it is coupled with symtab.
I wonder if moving it up in the enum would make it physically appear
next to .symtab in object file so we save a bit of i/o?
> +static void
> +produce_symtab_extension (struct output_block *ob)
> +{
> +  char *section_name = lto_get_section_name (LTO_section_symtab_extension,
> +                                          NULL, 0, NULL);
> +  lto_symtab_encoder_t encoder = ob->decl_state->symtab_node_encoder;
> +  lto_symtab_encoder_iterator lsei;
> +
> +  lto_begin_section (section_name, false);
> +  free (section_name);
> +
> +  /* Write the symbol table.
> +     First write everything defined and then all declarations.
> +     This is necessary to handle cases where we have duplicated symbols.  */
> +  for (lsei = lsei_start (encoder);
> +       !lsei_end_p (lsei); lsei_next (&lsei))
> +    {
> +      symtab_node *node = lsei_node (lsei);
> +
> +      if (DECL_EXTERNAL (node->decl) || !node->output_to_lto_symbol_table_p 
> ())
> +     continue;
> +      write_symbol_extension_info (node->decl);
> +    }
> +  for (lsei = lsei_start (encoder);
> +       !lsei_end_p (lsei); lsei_next (&lsei))
> +    {
> +      symtab_node *node = lsei_node (lsei);
> +
> +      if (!DECL_EXTERNAL (node->decl) || !node->output_to_lto_symbol_table_p 
> ())
> +     continue;
> +      write_symbol_extension_info (node->decl);
> +    }
> +
> +  lto_end_section ();
> +}

It seems fragile to me to duplicate the loops that needs to be kept in
sync because breaking that will likely get bit suprising outcome (i.e.
bootstrap will work since we do not care about symbol types). Perhaps it
would be more robust to simply push the extension bits into a vector in
write_symbol for later streaming.  Or at least add comment that loops
needs to be kept in sync.
> diff --git a/include/plugin-api.h b/include/plugin-api.h
> index 09e1202df07..804684449cf 100644
> --- a/include/plugin-api.h
> +++ b/include/plugin-api.h
> @@ -87,7 +87,17 @@ struct ld_plugin_symbol
>  {
>    char *name;
>    char *version;
> -  int def;
> +#ifdef __BIG_ENDIAN__
> +  char unused;
> +  char section_flags;
> +  char symbol_type;
> +  char def;
> +#else
> +  char def;
> +  char symbol_type;
> +  char section_flags;
> +  char unused;
> +#endif
Perhaps at least drop a comment why this is done this way (i.e.
compatibility with V3 version of API) so in 10 years we know?

Bit more systematic way would be to add a new hook to query extended
properties of a given symbol. I.e. something like
void *get_symbol_property (symbol, enum property)

Honza

Reply via email to