https://gcc.gnu.org/bugzilla/show_bug.cgi?id=123279

--- Comment #13 from GCC Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Robin Dapp <[email protected]>:

https://gcc.gnu.org/g:3a05a375defaf02838776dca13ca4992cf9d1c3d

commit r16-6974-g3a05a375defaf02838776dca13ca4992cf9d1c3d
Author: Robin Dapp <[email protected]>
Date:   Fri Jan 16 12:54:47 2026 +0100

    RISC-V: Correct builtin registration order [PR123279].

    When compiling ncnn with LTO we encountered an ICE in the final link
    step.  The reason for this is inconsistent builtin registration
    order, and, as a consequence, inconsistent functions codes being
    streamed.

    The underlying reason is that ncnn compiles files with different -march
    options, one of them being -march=..._xtheadvector.  XTheadVector does
    not support fractional LMULs and several insns while also adding new
    insns.  As we register builtins sequentially, not registering some
    builtins in one TU but registering them in another will naturally lead
    to different orders and incompatible function codes.

    I'm not really sure how such an executable is going to work eventually
    but we should not ICE at least.

    At first I tried to re-use the existing function_instance hash but that
    would quickly lead to collisions due to the high number of total
    builtins.  Linear probing for the next bucket would have caused the same
    problems we wanted to avoid in the first place.

    The problem with XTheadVector in particular is that it both takes away
    builtins (the ones with fractional LMUL) as well as adds its own.
    Therefore just partitioning the function-code space into extensions is
    not sufficient.  It would be sufficient if an extension only added
    builtins but the order will still be different if we just skip builtins
    with fractional LMUL.

    There are at least two options now:
     - Create placeholders for all skipped builtins.
     - Enable the unsupported builtins for XTheadVector and bail at expand
       time.

    In order to create placeholders we first need to get to the place where
    to create them.  As we have some guards for XTheadVector before that,
    verifying that types are available etc., the necessary changes would
    have touched several layers.

    Therefore I went with the second option above, combining it with
    partitioning the function space into extensions for a bit of future
    proofing.  Not creating placeholders is also in line with "polluting"
    the march flags, i.e. enable everything reasonably possible.

    To that end, the patch removes the TARGET_XTEADVECTOR-related checks
    in riscv-vector-switch.def and introduces a new builtin requirement
    VECTOR_EXT_NO_XTHEAD that contains the "V"-only but not XTHeadVector
    builtins.

    The function code now looks like this:

         Bit 0:       RISCV_BUILTIN_VECTOR (class bit)
         Bits 1-8:    Partition (rvv_builtin_partition enum)
         Bits 9+:     Index within partition.

    I tried to come up with a test case for quite a while but didn't manage.
    Reducing the ncnn LTO build failure also proved very difficult so in
    order to move forwarding I'm posting the patch without a dedicated test
    case.

            PR target/123279

    gcc/ChangeLog:

            * config/riscv/riscv-vector-builtins-functions.def
(REQUIRED_EXTENSIONS):
            Add VECTOR_EXT_NO_XTHEAD.
            (vlm): Move to VECTOR_EXT_NO_XTHEAD.
            (vsm): Ditto.
            (vzext): Ditto.
            (vsext): Ditto.
            (vaaddu): Ditto.
            (vaadd): Ditto.
            (vasubu): Ditto.
            (vasub): Ditto.
            (vfrsqrt7): Ditto.
            (vfrec7): Ditto.
            (vfrec7_frm): Ditto.
            (vfslide1up): Ditto.
            (vfslide1down): Ditto.
            (vluxei8): Ditto.
            (vluxei16): Ditto.
            (vluxei32): Ditto.
            (vluxei64): Ditto.
            (vsuxei8): Ditto.
            (vsuxei16): Ditto.
            (vsuxei32): Ditto.
            (vsuxei64): Ditto.
            (vluxseg): Ditto.
            (vsuxseg): Ditto.
            (vrgatherei16): Ditto.
            (vlseg): Keep.
            (vsseg): Keep.
            (vlsseg): Keep.
            (vssseg): Keep.
            (vloxseg): Keep.
            (vsoxseg): Keep.
            (vlsegff): Keep.
            * config/riscv/riscv-vector-builtins.cc (GTY): Split
            registered_functions into multiple partitions.
            (is_fractional_lmul): New function.
            (xthvector_unsupported_p): Ditto.
            (get_builtin_partition): Ditto.
            (function_builder::add_function): Use new functions.
            (lookup_registered_function): Ditto.
            (builtin_decl): Use lookup_registered_function.
            (gimple_fold_builtin): Ditto.
            (expand_builtin): Ditto.
            (check_builtin_call): Ditto.
            (resolve_overloaded_builtin): Ditto.
            * config/riscv/riscv-vector-builtins.h (enum required_ext):
            (enum rvv_builtin_partition): New enum.
            (required_ext_to_isa_name): Add NO_XTHEAD.
            (required_extensions_specified): Ditto.
            * config/riscv/riscv-vector-switch.def (ENTRY): Remove
            XTheadVector.
            (TUPLE_ENTRY): Ditto.
            * config/riscv/riscv.cc (riscv_expand_mult_with_const_int): Fix
            whitespace.
            (riscv_legitimize_poly_move): Ditto.
            (riscv_vector_fractional_lmul_p): New function.
            (riscv_validate_vector_type): Use new function.

Reply via email to