> Hi.
>
> There's updated version of the patch.
> Changes from the previous version:
> - comment added to ld_plugin_symbol
> - new section renamed to ext_symtab
> - assert added for loop iterations in produce_symtab and
> produce_symtab_extension
Hi,
I hope this is last version of the patch.
>
> 2020-03-12 Martin Liska <mli...@suse.cz>
>
> * lto-section-in.c: Add extension_symtab.
ext_symtab :)
> diff --git a/gcc/lto-section-in.c b/gcc/lto-section-in.c
> index c17dd69dbdd..78b015be696 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",
> + "ext_symtab"
I would move ext_symtab next to symtab so the sections remains at least
bit reasonably ordered.
>
> +/* Write extension information for symbols (symbol type, section flags). */
> +
> +static void
> +write_symbol_extension_info (tree t)
> +{
> + unsigned char c;
Do we still use vertical whitespace after decls per GNU coding style?
> diff --git a/gcc/lto-streamer.h b/gcc/lto-streamer.h
> index 25bf6c468f7..4f82b439360 100644
> --- a/gcc/lto-streamer.h
> +++ b/gcc/lto-streamer.h
> @@ -236,6 +236,7 @@ enum lto_section_type
> LTO_section_ipa_hsa,
> LTO_section_lto,
> LTO_section_ipa_sra,
> + LTO_section_symtab_extension,
I guess symtab_ext to match the actual section name?
> LTO_N_SECTION_TYPES /* Must be last. */
> };
>
> diff --git a/include/lto-symtab.h b/include/lto-symtab.h
> index 0ce0de10121..47f0ff27df8 100644
> --- a/include/lto-symtab.h
> +++ b/include/lto-symtab.h
> @@ -38,4 +38,16 @@ enum gcc_plugin_symbol_visibility
> GCCPV_HIDDEN
> };
>
> +enum gcc_plugin_symbol_type
> +{
> + GCCST_UNKNOWN,
> + GCCST_FUNCTION,
> + GCCST_VARIABLE,
> +};
> +
> +enum gcc_plugin_symbol_section_flags
> +{
> + GCCSSS_BSS = 1
> +};
Probably comments here?
> +
> #endif /* GCC_LTO_SYMTAB_H */
> +/* Parse an entry of the IL symbol table. The data to be parsed is pointed
> + by P and the result is written in ENTRY. The slot number is stored in
> SLOT.
> + Returns the address of the next entry. */
> +
> +static char *
> +parse_table_entry_extension (char *p, struct ld_plugin_symbol *entry)
> +{
> + unsigned char t;
> + enum ld_plugin_symbol_type symbol_types[] =
> + {
> + LDST_UNKNOWN,
> + LDST_FUNCTION,
> + LDST_VARIABLE,
> + };
> +
> + t = *p;
> + check (t <= 3, LDPL_FATAL, "invalid symbol type found");
> + entry->symbol_type = symbol_types[t];
> + p++;
> + entry->section_flags = *p;
> + p++;
> +
> + return p;
> +}
I think we have chance to make some plan for future extensions without
introducing too many additional sections.
Currently there are 2 bytes per entry, while only 3 bits are actively
used of them. If we invent next flag to pass we can use unused bits
however we need a way to indicate to plugin that the bit is defined.
This could be done by a simple version byte at the beggining of
ext_symtab section which will be 0 now and once we define extra bits we
bump it up to 1.
It is not that important given that even empty file results in 2k LTO
object file, but I think it would be nicer in longer run.
> + /* This is for compatibility with older ABIs. */
Perhaps say here that this ABI defined only "int def;"
The patch look good to me. Thanks for the work!
Honza
> +#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
> int visibility;
> uint64_t size;
> char *comdat_key;
> @@ -123,6 +134,20 @@ enum ld_plugin_symbol_visibility
> LDPV_HIDDEN
> };
>
> +/* The type of the symbol. */
> +
> +enum ld_plugin_symbol_type
> +{
> + LDST_UNKNOWN,
> + LDST_FUNCTION,
> + LDST_VARIABLE,
> +};
> +
> +enum ld_plugin_symbol_section_flags
> +{
> + LDSSS_BSS = 1
> +};
> +
> /* How a symbol is resolved. */
>
> enum ld_plugin_symbol_resolution
> @@ -431,7 +456,9 @@ enum ld_plugin_tag
> LDPT_GET_INPUT_SECTION_ALIGNMENT = 29,
> LDPT_GET_INPUT_SECTION_SIZE = 30,
> LDPT_REGISTER_NEW_INPUT_HOOK = 31,
> - LDPT_GET_WRAP_SYMBOLS = 32
> + LDPT_GET_WRAP_SYMBOLS = 32,
> + LDPT_ADD_SYMBOLS_V2 = 33,
> + LDPT_GET_SYMBOLS_V4 = 34,
> };
>
> /* The plugin transfer vector. */
> --
> 2.25.1
>
