Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

compiler-rt: Provide __cpu_indicator_init, __cpu_model and __cpu_features2 #20081

Open
wants to merge 11 commits into
base: master
Choose a base branch
from

Conversation

lacc97
Copy link
Contributor

@lacc97 lacc97 commented May 26, 2024

Fixes #18074, original PR was #18193.

This PR adds support for X86/X86_64 CPU model detection and function multiversioning in C/C++ via the target and target_clones attributes (compatible with LLVM's, which in turn is compatible with libgcc's). It adds __cpu_indicator_init, __cpu_model and __cpu_features2 symbols to compiler-rt for x86/x86_64 which reuse the CPU detection logic under std/zig/system/x86.zig and transform the detected features into the values expected by LLVM-generated code. These values are kept in sync with LLVM using a new tool (tools/update_x86_cpu_model_enums.zig) which parses llvm/include/llvm/TargetParser/X86TargetParser.def in LLVM to generate the corresponding Zig enums.

This PR also adds a standalone test that the symbols are emitted when the target attribute is used in C.

While this functionality could in principle contribute to solving issue #4591 (edit: wrong issue, its #1018), I think it would be best to use a Zig-specific solution for richer CPU detection capabilities, leveraging std.Target.* instead of relying only on the feature set supported by LLVM/libgcc. The changes in this PR would therefore only be useful for C/C++ compatibility with Clang and gcc.

@daurnimator
Copy link
Contributor

I could have sworn there was an issue about function multiversioning/selection based on runtime-detected CPU in Zig itself but I can't find it now

#1018 ?

@lacc97
Copy link
Contributor Author

lacc97 commented May 29, 2024

I could have sworn there was an issue about function multiversioning/selection based on runtime-detected CPU in Zig itself but I can't find it now

#1018 ?

Yep that was it. Thanks.

@theseyan
Copy link

theseyan commented Nov 3, 2024

Missing support for these symbols is a blocker for compiling libmdbx (a fork of LMDB) with Zig:

error: ld.lld: undefined symbol: __cpu_indicator_init
    note: referenced by gc-get.c:584 (src/gc-get.c:584)
    note:               mdbx-static.o:(scan4seq_resolver) in archive [...]/libmdbx.a
error: ld.lld: undefined symbol: __cpu_model
    note: referenced by gc-get.c:587 (src/gc-get.c:587)
    note:               mdbx-static.o:(scan4seq_resolver) in archive [...]/libmdbx.a

I really appreciate this PR and hope it gets merged soon. 🫡

@slyshykO
Copy link

slyshykO commented Nov 3, 2024

As a temporary solution, you can use cpu_model, which provides __cpu_indicator_init and __cpu_model symbols.

@lacc97 lacc97 force-pushed the compiler-rt-x86-cpu branch from 577394c to 167cb40 Compare January 19, 2025 02:27
@lacc97
Copy link
Contributor Author

lacc97 commented Jan 19, 2025

Rebased to latest master.

lacc97 added 10 commits January 19, 2025 16:26
These match what LLVM expects and generates when building multiversioned
functions (e.g. functions tagged with attributes target or
target_clones). The actual CPU detection reuses the logic under
std/zig/system/x86.zig, and transforms it into the values that
LLVM-generated code expects. These values are auto-generated using a
new tool (tools/update_x86_cpu_model_enums.zig) which parses the
relevant file in the LLVM codebase and generates Zig enums out of it.
- Use pointers in export builtin
- Update names for Type enum tag
@lacc97 lacc97 force-pushed the compiler-rt-x86-cpu branch from 167cb40 to a481fa8 Compare January 19, 2025 17:00
@lacc97
Copy link
Contributor Author

lacc97 commented Jan 19, 2025

Rebased again . I'm not entirely happy with getAmdTypeAndSubtype() and getIntelTypeAndSubtype() as it seems to duplicate similar logic as in lib/std/zig/system/x86.zig. It's not clear to me how to resolve this without bringing in the generated enum types inside the standard library, which don't really fit there.

The new logic is closer to how it's done in C where any unspecified
enum simply increments the previous value.
@alexrp
Copy link
Member

alexrp commented Jan 26, 2025

It's not clear to me how to resolve this without bringing in the generated enum types inside the standard library, which don't really fit there.

Is it actually problematic if the standard library @imports a file from compiler-rt or vice versa? If it just contains data, as seems to be the case, that seems to me like it'd be harmless?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Missing __cpu_indicator_init and __cpu_model in compiler-rt
5 participants