Add support for target details (CPUs and their supported features) #4264

andrewrk · 2020-01-22T03:35:08Z

This is @layneson's pull request #3927, merged into a branch, and then a bunch of my commits piled on top of it, with several major changes:

Using a bit set for CPU features, to avoid having std.Target ever require heap allocation
Adding the concept of "baseline" CPU features into Zig and exposing it, rather than implicitly relying on LLVM for this.
Exposing the set of CPU features to builtin.zig so that comptime code has access to the information.
Moving zig targets entirely into userland, and implementing make zig targets emit well-formed json #4213 (btw @LemonBoy I experimented with piping the output into jq and it works quite well)

This is complete and ready to be merged into master. The only thing left is getting the CI green.

Previously, buffers were used with toOwnedSlice() to create c strings for LLVM cpu/feature strings. However, toOwnedSlice() shrinks the string memory to the buffer's length, which cuts off the null terminator. Now toSliceConst() is used instead, and the buffer is not deinited so that the string memory is not freed.

see BRANCH_TODO file

to avoid an illegal instruction error with the older qemu version that is available on the CI server.

See #508. These can be re-enabled when we upgrade to LLVM 10.

comment from this commit reproduced here: I have observed the CPU name reported by LLVM being incorrect. On the SourceHut build services, LLVM 9.0 reports the CPU as "athlon-xp", which is a 32-bit CPU, even though the system is 64-bit and the reported CPU features include, among other things, +64bit. So the strategy taken here is that we observe both reported CPU, and the reported CPU features. The features are trusted more; but if the features match exactly the features of the reported CPU, then we trust the reported CPU.

This reverts commit 4640ef5. This attempted workaround did not have the desired effect.

daurnimator · 2020-01-22T03:45:52Z

lib/std/target.zig

@@ -496,7 +864,7 @@ pub const Target = union(enum) {
    pub fn parseArchSub(text: []const u8) ParseArchSubError!Arch {
        const info = @typeInfo(Arch);
        inline for (info.Union.fields) |field| {
-            if (mem.eql(u8, text, field.name)) {
+            if (mem.startsWith(u8, text, field.name)) {


Why startsWith instead of eql?

example: aarch64v8_5a-linux-musl

related: #4261

src/main.cpp

andrewrk · 2020-01-22T03:54:31Z

It looks like there is a problem with determining what CPU features to report to LLVM for the "baseline" CPUs of aarch64. @LemonBoy if you are interested in this, I could really use your expertise here.

std/target.zig

        /// The "default" set of CPU features for cross-compiling. A conservative set
        /// of features that is expected to be supported on most available hardware.
        pub fn baselineFeatures(arch: Arch) Cpu.Feature.Set {
            return switch (arch) {
                .arm, .armeb, .thumb, .thumbeb => arm.cpu.generic.features,
                .aarch64, .aarch64_be, .aarch64_32 => aarch64.cpu.generic.features,
                .avr => avr.baseline_features,
                .bpfel, .bpfeb => bpf.cpu.generic.features,
                .hexagon => hexagon.cpu.generic.features,
                .mips, .mipsel => mips.cpu.mips32.features,
                .mips64, .mips64el => mips.cpu.mips64.features,
                .msp430 => msp430.cpu.generic.features,
                .powerpc, .powerpc64, .powerpc64le => powerpc.cpu.generic.features,
                .amdgcn => amdgpu.cpu.generic.features,
                .riscv32 => riscv.baseline_32_features,
                .riscv64 => riscv.baseline_64_features,
                .sparc, .sparcv9, .sparcel => sparc.cpu.generic.features,
                .s390x => systemz.cpu.generic.features,
                .i386 => x86.cpu.pentium4.features,
                .x86_64 => x86.cpu.x86_64.features,
                .nvptx, .nvptx64 => nvptx.cpu.sm_20.features,
                .wasm32, .wasm64 => wasm.cpu.generic.features,

                else => Cpu.Feature.Set.empty,
            };
        }

I tried to make this match what we are doing in master branch, but I must have done something wrong for aarch64. There's a new flag --verbose-llvm-cpu-features to help troubleshoot this. For master branch it would be reporting an empty string (leaving it to default to llvm); in this branch we take matters into our own hands and always specify a set of CPU features to LLVM.

andrewrk · 2020-01-22T05:45:56Z

I'm pretty sure every CI failure comes down to using a different string for llvm_target_cpu_features to createTargetMachine than is passed in master (empty string for cross compiling, directly passing the result of ZigLLVMGetTargetFeatures() for native).

But rather than reverting to that behavior, the goal is to improve zig's target cpu features capabilities such that it can expose the options at comptime, as well as give the correct string to LLVM.

daurnimator · 2020-01-22T09:55:43Z

lib/std/target/riscv.zig

+    };
+    result[@enumToInt(Feature.e)] = .{
+        .llvm_name = "e",
+        .description = "Implements RV32E (provides 16 rather than 32 GPRs)",


Should RV32I/RV32E/RV64I/RV128I be sub-architectures rather than features?
CC @xobs

related: #4261

lib/std/build.zig

LemonBoy · 2020-01-22T14:04:19Z

Ok so, here's some good news. The problem with the sr.ht servers are due to the fact that the cpu parameter is set to qemu64 (see here) because FreeBSD apparently has some issues with the host cpu.

You can reproduce the problem with qemu user-mode emulation for x86_64:

qemu-x86_64 -cpu qemu64 ~/code/zig/build/zig0 --verbose-llvm-cpu-features build-exe --cache off ~/code/zig/test/standalone/hello_world/hello.zig

This reports

target_specific_cpu_args=athlon-xp

You can also double-check how qemu defines the qemu64 CPU.

The commit that worked around this problem can be reverted and the sr.ht CI script amended to pick a suitable target.

Edit: Here's the upstream bug report.

andrewrk · 2020-01-22T16:42:52Z

The commit that worked around this problem can be reverted and the sr.ht CI script amended to pick a suitable target.

This would result in Zig failing to compile anything on that platform, unless the user worked around this issue. That's not right; zig should be able to compile code in this environment without a workaround hack done by the user. LLVM is incorrect - the CPU is not a (32-bit!) athlon-xp, despite whatever config the OS has saying otherwise.

I'm also concerned about LLVM's reports of cpu features on one of my laptops. It reports "skylake" for the CPU, which is correct, but then it lists the features as:

"64bit",
"adx",
"aes",
"avx",
"avx2",
"bmi",
"bmi2",
"clflushopt",
"cmov",
"cx16",
"cx8",
"f16c",
"fma",
"fsgsbase",
"fxsr",
"invpcid",
"lzcnt",
"macrofusion",
"mmx",
"movbe",
"mpx",
"nopl",
"pclmul",
"popcnt",
"prfchw",
"rdrnd",
"rdseed",
"rtm",
"sahf",
"sgx",
"slow_3ops_lea",
"slow_incdec",
"sse",
"sse2",
"sse3",
"sse4_1",
"sse4_2",
"ssse3",
"x87",
"xsave",
"xsavec",
"xsaveopt",
"xsaves",

But the list of skylake features is:

"64bit",
"adx",
"aes",
"avx",
"avx2",
"bmi",
"bmi2",
"clflushopt",
"cmov",
"cx16",
"cx8",
"ermsb",
"f16c",
"false_deps_popcnt",
"fast_gather",
"fast_scalar_fsqrt",
"fast_shld_rotate",
"fast_variable_shuffle",
"fast_vector_fsqrt",
"fma",
"fsgsbase",
"fxsr",
"idivq_to_divl",
"invpcid",
"lzcnt",
"macrofusion",
"merge_to_threeway_branch",
"mmx",
"movbe",
"mpx",
"nopl",
"pclmul",
"popcnt",
"prfchw",
"rdrnd",
"rdseed",
"sahf",
"sgx",
"slow_3ops_lea",
"sse4_2",
"x87",
"xsave",
"xsavec",
"xsaveopt",
"xsaves",

Which has more things in it than what was reported as available CPU features. So presumably it would be incorrect to depend on, for example, fast_gather, based on the fact that the CPU name is skylake, and skylake has that feature, because for some reason LLVM has detected that the native host does not support that feature.

Previously it was a tagged union which was one of: * baseline * a specific CPU * a set of features Now, it's possible to have a CPU but also modify the CPU's feature set on top of that. This is closer to what LLVM does. This is more correct because Zig's notion of CPUs (and LLVM's) is not exact CPU models. For example "skylake" is not one very specific model; there are several different pieces of hardware that match "skylake" that have different feature sets enabled.

hopefully this avoids the older qemu version crashing

andrewrk · 2020-01-23T06:50:10Z

asking for help on llvm-dev mailing list

These were never working with native CPU features. In this branch, we fix native CPU features not being enabled on Windows, and regress f128 language features. In the llvm10 branch, all this is fixed, and the tests are re-enabled.

tests use older sub-arch that works in the older qemu

…eatures

in stack tracing code, the idea was to detect the tty settings at the top of the stack and pass the information down. somewhere along the way this got changed so that setTtyColor was assuming the global stderr_file was related to the output stream the stack trace was being printed to. now, tty_color is changed to tty_config, and it is an enum rather than a bool, telling how tty colors are expected to be handled. windows is still incorrectly looking at stderr_file.

layneson and others added 30 commits January 19, 2020 20:53

Create initial target details infrastructure

0f46c12

Update term feature deps -> subfeatures

8f191e0

Add parseArchTag and fix parseArchSub

8ac138a

Fix CPU and feature defs

21908e1

Make targets cmd able to list CPUs and features

5bc4690

Fix spacing in main.cpp

9d66bda

Switch CPU/features to simple format

c131e50

Remove llvm_name from features

c8f1e0d

Add cpu/feature specification to cmndline

bd6ef21

Add cpu/feature to cache hash

b3324f1

Add build.zig cpu and feature options

c1798cb

Filter out non-features

5137220

Rename subfeatures -> dependencies

e4ecdef

Add llvm_name to feature defs

79a2747

Add TargetDetails abstraction

c61856e

Add builtin.zig support

03dd376

Add defaut feature support

fd17a99

Make sure llvm strings are null-terminated

ebb6f15

Only enable requested features

40ff359

No allocations for n.t. empty strings

a5c9397

Remove features/cpus not in LLVM v9

de8a5cf

Enable 64bit feature for riscv64

8902f3c

Pass target details to c compiler

62e4cc0

Allow target details with no LLVM support

430077d

Pass target_details to child CodeGens

c156234

progress towards merging

a867b43

see BRANCH_TODO file

figure out zig0/stage1 and scanning for native CPU

a313f15

do the x86 arch

e3b5e91

some fixes

20af858

andrewrk added 4 commits January 21, 2020 21:02

tests: use an older aarch64 sub-arch

4640ef5

to avoid an illegal instruction error with the older qemu version that is available on the CI server.

enable native CPU feature for windows; disable failing tests

830e0ba

See #508. These can be re-enabled when we upgrade to LLVM 10.

Revert "tests: use an older aarch64 sub-arch"

c6bfece

This reverts commit 4640ef5. This attempted workaround did not have the desired effect.

andrewrk mentioned this pull request Jan 22, 2020

Add support for target details (CPUs and their supported features) #3927

Closed

daurnimator reviewed Jan 22, 2020

View reviewed changes

LemonBoy mentioned this pull request Jan 22, 2020

-target armv6m-freestanding-none does not compile #4266

Open

daurnimator reviewed Jan 22, 2020

View reviewed changes

lib/std/build.zig Outdated Show resolved Hide resolved

daurnimator mentioned this pull request Jan 22, 2020

Add zig example im-tomu/fomu-workshop#140

Merged

andrewrk added 5 commits January 22, 2020 17:13

fix not respecting sub-arch feature

3227aec

fix std.Target.Arch.parseCpuFeatureSet unit test

0c477f3

aarch64: less feature-full baseline CPU

9845264

use an older arm64 sub-arch for test suite

ead7d15

hopefully this avoids the older qemu version crashing

andrewrk added 2 commits January 23, 2020 02:05

disable f128 compiler_rt tests failing on windows

c86589a

These were never working with native CPU features. In this branch, we fix native CPU features not being enabled on Windows, and regress f128 language features. In the llvm10 branch, all this is fixed, and the tests are re-enabled.

fix incorrect list of sub-arches for aarch64

fbfda7f

tests use older sub-arch that works in the older qemu

andrewrk added the breaking Implementing this issue could cause existing code to no longer compile or have different behavior. label Jan 23, 2020

andrewrk mentioned this pull request Jan 26, 2020

split IrInstruction into IrInst, IrInstSrc, IrInstGen #4290

Merged

andrewrk added 3 commits January 25, 2020 23:25

Merge remote-tracking branch 'origin/master' into layneson-cpus_and_f…

9dffc36

…eatures

fix compilation error

d9fb6c2

andrewrk merged commit 96e5f47 into master Jan 26, 2020

andrewrk deleted the layneson-cpus_and_features branch January 26, 2020 14:57

frmdstryr mentioned this pull request Feb 19, 2020

remove the concept of "sub-architecture" in favor of CPU features #4261

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add support for target details (CPUs and their supported features) #4264

Add support for target details (CPUs and their supported features) #4264

andrewrk commented Jan 22, 2020 •

edited

daurnimator Jan 22, 2020

andrewrk Jan 22, 2020 •

edited

andrewrk commented Jan 22, 2020 •

edited

andrewrk commented Jan 22, 2020 •

edited

daurnimator Jan 22, 2020

andrewrk Jan 22, 2020

LemonBoy commented Jan 22, 2020 •

edited

andrewrk commented Jan 22, 2020

andrewrk commented Jan 23, 2020

Add support for target details (CPUs and their supported features) #4264

Add support for target details (CPUs and their supported features) #4264

Conversation

andrewrk commented Jan 22, 2020 • edited

daurnimator Jan 22, 2020

Choose a reason for hiding this comment

andrewrk Jan 22, 2020 • edited

Choose a reason for hiding this comment

andrewrk commented Jan 22, 2020 • edited

andrewrk commented Jan 22, 2020 • edited

daurnimator Jan 22, 2020

Choose a reason for hiding this comment

andrewrk Jan 22, 2020

Choose a reason for hiding this comment

LemonBoy commented Jan 22, 2020 • edited

andrewrk commented Jan 22, 2020

andrewrk commented Jan 23, 2020

andrewrk commented Jan 22, 2020 •

edited

andrewrk Jan 22, 2020 •

edited

andrewrk commented Jan 22, 2020 •

edited

andrewrk commented Jan 22, 2020 •

edited

LemonBoy commented Jan 22, 2020 •

edited