The 09/07/2025 12:41, Jeff Law wrote: > > > On 8/28/25 3:49 AM, [email protected] wrote: > > From: Alfie Richards <[email protected]> > > > > 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 (cgraph_node::insert_new_function_version): Record > > assembler_name. > > * cgraph.h (struct cgraph_function_version_info): Add assembler_name. > > (struct cgraph_node): Add dispatcher_resolver_function and > > is_target_clone. > > * 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 > > (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. > > (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): Removed. > > (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. > Privately Alfie indicated to me this is ready to go except for aarch64 & > riscv review. While I don't usually get into aarch64 code, the bulk of it > is just adjusting for the change to use the slice API and another big blob > is effectively the same as the risc-v changes.
Hi Jeff, Thank you for the review! Apologies though, it seems wires were crossed, It is the x86 (and indeed the risc-v) changes that still needed review. (Richard S's review of Aarch64 changes previously: https://gcc.gnu.org/pipermail/gcc-patches/2025-July/691288.html) The x86 change is of most importance to me as it changes the mangling for the FMV symbols (see the test changes). Apologies for the confusion, Thank you again, Alfie > > With that. This is fine for the trunk. > > Thanks for your patience. > > jeff > -- Alfie Richards
