This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH 03/25] Improve TARGET_MANGLE_DECL_ASSEMBLER_NAME.
On Wed, 12 Sep 2018 13:34:06 -0400
Julian Brown <julian@codesourcery.com> wrote:
> On Wed, 12 Sep 2018 17:31:58 +0100
> Andrew Stubbs <ams@codesourcery.com> wrote:
>
> > On 12/09/18 16:16, Richard Biener wrote:
> > > I think the symptom GCN sees needs to be better understood - like
> > > wheter it is generally OK to mangle things arbitrarily.
> >
> > The name mangling is a horrible workaround for a bug in the HSA
> > runtime code (which we do not own, cannot fix, and would want to
> > support old versions of anyway). Basically it refuses to load any
> > binary that has the same symbol as another, already loaded, binary,
> > regardless of the symbol's linkage. Worse, it also rejects any
> > binary that has duplicate symbols within it, despite the fact that
> > it already linked just fine.
> >
> > Adding the extra lookups is enough to build GCN binaries, with
> > mangled names, whereas the existing name mangling support was either
> > more specialized or bit rotten (I don't know which).
> >
> > It may well be that there's a better way to solve the problem, or
> > at least to do the lookups.
> >
> > It may also be that there are some unintended consequences, such as
> > false name matches, but I don't know of any at present.
> >
> > Julian, can you comment, please?
>
> I did the local-symbol name mangling in two places:
>
> - The ASM_FORMAT_PRIVATE_NAME macro (good for local statics)
> - The TARGET_MANGLE_DECL_ASSEMBLER_NAME hook (for file-scope
> local/statics)
>
> Possibly, this was an abuse of these hooks, but it's arguably wrong
> that that e.g. handle_alias_pairs has the "assembler name" leak
> through into the user's source code -- if it's expected that the hook
> could make arbitrary transformations to the string. (The latter hook
> is only used by PE code for x86 at present, by the look of it, and
> the default handles only special-purpose mangling indicated by
> placing a '*' at the front of the symbol.)
One possibility might be to allow
symbol_table::decl_assembler_name_hash and
symbol_table::assembler_names_equal_p to be overridden by a target
hook, and define them for GCN to ignore the symbol "localisation"
magic. At the moment they will just ignore "*" at the start of a
symbol, and a (fixed) user label prefix, no matter what
TARGET_MANGLE_DECL_ASSEMBLER_NAME does.
Another way would be to do some appropriate mangling for local symbols
in the assembler, rather than the compiler (though we're using the LLVM
assembler, and so far have got away with not making any invasive
changes to that).
> If we had a symtab_node::get_for_name () using a suitable hash table,
> I think it'd probably be right to use that. Can that be done
> (easily), or is there some equivalent way? Introducing a new hash
> table everywhere for a bug workaround for a relatively obscure
> feature on a single target seems unfortunate.
An "obvious" solution of calling targetm.mangle_decl_assembler_name
before looking up in symtab_node::get_for_asmname, something like:
static void
handle_alias_pairs (void)
{
alias_pair *p;
unsigned i;
for (i = 0; alias_pairs && alias_pairs->iterate (i, &p);)
{
tree asmname = targetm.mangle_decl_assembler_name (p->decl, p->target);
symtab_node *target_node = symtab_node::get_for_asmname (asmname);
[...]
seems like it could possibly work for handle_alias_pairs, but not so
much for c-pragma.c:maybe_apply_pending_pragma_weaks, where there is no
decl available to pass as the first argument to the target hook.
Julian