On 26/07/2025 14:27, Jeff Law wrote:


On 6/12/25 10:53 AM, Alfie Richards wrote:
This patch is an overhaul of how FMV name mangling works. Previously
mangling logic was duplicated in several places across both target
specific and independent code. This patch changes this such that all
mangling is done in targetm.mangle_decl_assembler_name (including for the
dispatched symbol and dispatcher resolver).

Adds the assembler_name member to cgraph_function_version_info to store
the base assembler name of the function set, before FMV mangling.

This allows for the removing of previous hacks, such as where the default
mangled decl's assembler name was unmangled to then remangle all versions
and the resolver and dispatched symbol.

This introduces a change (shown in test changes) for the assembler name of the dispatched symbol for a x86 versioned function set. Previously it used the
function name mangled twice. This was hard to reproduce without hacks I
wasn't comfortable with. Therefore, the mangling is changed to instead append
".ifunc" which matches clang's behavior.

This change also refactors expand_target_clone using
targetm.mangle_decl_assembler_name for mangling and get_clone_versions.
It is modified such that if the target_clone is in a FMV structure
the ordering is preserved once expanded. This is used later for ACLE semantics
and target_clone/target_version mixing.

gcc/ChangeLog:

    * attribs.cc (make_dispatcher_decl): Move duplicated cgraph logic into     this function and change to use targetm.mangle_decl_assembler_name for
    mangling.
    * cgraph.cc (delete_function_version): Made public static member of
    cgraph_node.
    (cgraph_node::insert_new_function_version): Record assembler_name.
    * cgraph.h (delete_function_version): Ditto.
    (struct cgraph_function_version_info): Add assembler_name.
    * config/aarch64/aarch64.cc (aarch64_parse_fmv_features): Change to
    support string_slice.
    (aarch64_process_target_version_attr): Ditto.
    (get_feature_mask_for_version): Ditto.
    (aarch64_mangle_decl_assembler_name): Add logic for mangling dispatched
    symbol and resolver.
    (get_suffixed_assembler_name): Removed.
    (make_resolver_func): Refactor to use
    aarch64_mangle_decl_assembler_name for mangling.
    (aarch64_generate_version_dispatcher_body): Remove remangling.
    (aarch64_get_function_versions_dispatcher): Refactor to remove
    duplicated cgraph logic.
    * config/i386/i386-features.cc (is_valid_asm_symbol): Moved from
    multiple_target.cc.
    (create_new_asm_name): Ditto.
    (ix86_mangle_function_version_assembler_name): Refactor to use
    clone_identifier and to mangle default.
    (ix86_mangle_decl_assembler_name): Add logic for mangling dispatched
    symbol and resolver.
    (ix86_get_function_versions_dispatcher): Remove duplicated cgraph
    logic.
    (make_resolver_func): Refactor to use ix86_mangle_decl_assembler_name
    for mangling.
    * config/riscv/riscv.cc (riscv_mangle_decl_assembler_name): Add logic
    for FMV mangling.
    (get_suffixed_assembler_name): Removed.
    (make_resolver_func): Refactor to use riscv_mangle_decl_assembler_name
    for mangling.
    (riscv_generate_version_dispatcher_body): Remove unnecessary remangling.
    (riscv_get_function_versions_dispatcher): Remove duplicated cgraph
    logic.
    * config/rs6000/rs6000.cc (rs6000_mangle_decl_assembler_name): New
    function.
    (rs6000_get_function_versions_dispatcher): Remove duplicated cgraph
    logic.
    (make_resolver_func): Refactor to use rs6000_mangle_decl_assembler_name
    for mangling.
    (is_valid_asm_symbol): Move from multiple_target.cc.
    (create_new_asm_name): Ditto.
    (rs6000_mangle_function_version_assembler_name): New function.
    * multiple_target.cc (create_dispatcher_calls): Remove mangling code.
    (get_attr_str): Removed.
    (separate_attrs): Ditto.
    (is_valid_asm_symbol): Moved to target specific.
    (create_new_asm_name): Ditto.
    (expand_target_clones): Refactor to use
    targetm.mangle_decl_assembler_name for mangling and be more general.
    * tree.cc (get_target_clone_attr_len): Removed.
    * tree.h (get_target_clone_attr_len): Removed.

gcc/cp/ChangeLog:

    * decl.cc (maybe_mark_function_versioned): Change to insert function version
    and therefore record assembler name.

gcc/testsuite/ChangeLog:

    * g++.target/i386/mv-symbols1.C: Update x86 FMV mangling.
    * g++.target/i386/mv-symbols3.C: Ditto.
    * g++.target/i386/mv-symbols4.C: Ditto.
    * g++.target/i386/mv-symbols5.C: Ditto.

Add assembler_name to cgraph_function_version_info.
Sorry things stalled out a bit.  Hopefully we can get through these larger patches and get this moving again.
No problem. Thank you for the review.>
So it's unclear why is_valid_asm_symbol and create_new_asm_name moved out of multiple-target.cc and into the target files themselves.  The set of characters allowed by the former seems like the minimial set that a target must support to handle C/C++ code.  Just not sure why that code got unfactored.

For create_new_asm_name it looks like it still does the same thing in all 3 copies, so again, unclear why it got unfactored.  Maybe I missed a subtle change here.
It's more of a stylistic change than a practical one. As FMV mangling seems to be target specific, I was moving mangling logic to be target specific. But that does look a little over enthusiastic. Happy to move it back if you prefer?


I don't see any obvious dependencies on later patches, so if this patch can go in now rather than waiting, then it probably should.  That gives it soak time in the event I can't get through the rest of the kit relatively quickly.
Yeah this can go in now. I am happy to commit this after approval.


jeff

Reply via email to