On Wed, 12 Sep 2018 17:31:58 +0100 Andrew Stubbs <a...@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.) I couldn't find an existing place where the DECL_NAMEs for symbols were indexed in a hash table, equivalent to the table for assembler names. Aliases are made via pragmas, so it's not 100% clear to me what the scoping/lookup rules are supposed to be for those anyway, nor what the possibility or consequences might be of false matches. (The "!target" case in maybe_apply_pending_pragma_weaks, if it doesn't somehow make a false match, just slows down reporting of an error a little, I think. Similarly in handle_alias_pairs.) 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. Thanks, Julian
