ziglang / zig

General-purpose programming language and toolchain for maintaining robust, optimal, and reusable software.
https://ziglang.org
MIT License
33.39k stars 2.43k forks source link

zig cc: parse `-target` and `-mcpu`/`-march`/`-mtune` flags according to clang #4911

Closed andrewrk closed 1 day ago

andrewrk commented 4 years ago

Extracted from #4784

Status quo:

Even when doing zig cc, -target is parsed as Zig's -target flag, which is different in some ways than clang's, as it contains (optional) os version info, glibc version info, and it's proposed (#4584) to have cpu feature syntax as well. Similarly, -mcpu, -march, and -mtune are all treated identically, as the zig -mcpu parameter. This is useful since the zig syntax allows specifying CPU model name and features.

This proposal is to add more code to support these options to be strictly compliant with clang's syntax. This would potentially improve the "drop-in" robustness of this feature, at the expense of the extra info that zig's syntax supports. If this proposal is implemented, then zig cc probably would need to introduce a few of its own non-standard flags to support specifying this additional information.

As an argument for not doing this, it's simpler and more powerful to leave things as status quo, and often, the reason people are using zig cc is for cross compiling anyway, in which case they will be putting the -target and/or -mcpu flags as part of the CC environment variable or equivalent. For this use case status quo is actually preferable.

mikdusan commented 3 years ago

If we advertise as "drop in replacement" then I would side with parse-as-clang-does with one exception. We are permitted to add zig variants of options where it makes sense:

A compelling case was reported by Justin Whear on discord where use of zig cc (as a wrapper script) and cmake detects clang, and wants to pass -march=core-avx2 which fails because currently zig cc does not recognize core-avx2 (but does recognize core_avx2).

jwhear commented 3 years ago

Thanks to @mikdusan for adding my use case to this issue. I thought I'd follow up with a few specifics to reinforce his suggestion.

The background is that I'm trying to compile the Intel Hyperscan library with zig cc and zig c++. I ran into issues using the default build due to missing symbols from the C++ std library while linking against my Zig code: whatever Zig is doing with -lc++ seems to not link the same thing that GCC is doing when it builds Hyperscan. I understand that Zig is using LLVM under the hood and possibly only compiling the bits of the std library it needs. I figured rather than installing Clang and figuring out how to get the versions to align nicely with Zig, I'd just use Zig as my compiler for the whole shebang.

So I created simple zig-cc and zig-c++ scripts in my path that invoke the equivalent zig commands and pass all arguments along (CMake needs paths not commands for the compiler variables). I then tried configuring Hyperscan using cmake .. -D CMAKE_C_COMPILER='zig-cc' -D CMAKE_CXX_COMPILER='zig-c++' but this failed as the project relies on features like SSSE3 and AVX2. My CPU has those features so I went down the rabbit hole of figuring out why the CMake checks for them failed.

It appears that CMake recognizes zig cc and zig c++ as Clang 12 (cool!) and then queries for feature support by trying to compile tiny source files to certain target architectures. In the particular failure that I encountered it was specifying -march=core-avx2 (note the dash). Clang and GCC recognize this as a valid target but, due to the renaming here, Zig does not recognize it and throws an error. I don't know what the rationale was for replacing all dashes with underscores in target names, but the result is that it breaks this kind of drop-in use. Obviously if this were my own build script it'd be a trivial change, but in this case either CMake needs to add Zig support or we need to more fully impersonate Clang.

jwhear commented 3 years ago

Just in case anyone is trying to do something similar, I hacked around this for the time being by making my wrapper scripts do a bit of translation work:

#!/bin/bash

# See https://github.com/ziglang/zig/issues/4911
args=""
for arg in "$@"
do
    parg="$arg"

    option=${arg%=*}
    target=${arg#*=}
    if [[ $option == "-march" || $option == "-mcpu" || $option == "-mtune" ]]; then
        fixedTarget=${target//-/_}
        parg="$option=$fixedTarget"
    fi
    args="$args $parg"
done

zig c++ $args
orent commented 1 year ago

Targets accepted by zig clang but not by zig cc

generic goldmont-plus core-avx-i core-avx2 skylake-avx512 icelake-client icelake-server athlon-fx x86-64

BTW, neither accepts the targets x86-64-v2, x86-64-v3, x86-64-v4 that appear in the target list.

While targets are usually chosen quite deliberately and specifically, there is one that is sprinkled onto many makefiles and build scripts and that is -mtune=generic. It is quite common for this option to be the only thing that breaks a build where zig cc would otherwise serve as a drop-in replacement for gcc.

andrewrk commented 1 year ago

generic is in fact recognized by zig cc. The others are named with underscores instead of dashes.

orent commented 1 year ago

Was this fixed since 0.9.1?

anagram3k commented 1 year ago

generic is in fact recognized by zig cc. The others are named with underscores instead of dashes.

I know @andrewrk is the creator of zig, but generic is not recognized by zig cc, only by zig clang. This is not yet fixed on 0.11.0-dev.174

Considering this answer on stackoverflow, probably it is best to just delete the '-mtune' option to bypass this bug.

Milo123459 commented 1 year ago

Are there any updates on this?

breezewish commented 4 months ago

Another one interesting case related with this issue, preventing us using zig just as a drop-in replacement of clang:

Works:

clang++ --target=aarch64-apple-darwin -march=armv8+crc+simd 1.cc

Fail:

zig c++ -target aarch64-macos-none -march=armv8+crc+simd 1.cc

info: available CPUs for architecture 'aarch64':
 a64fx
 ampere1
 ampere1a
 apple_a10
 apple_a11
 apple_a12
 apple_a13
 apple_a14
 apple_a15
 apple_a16
 apple_a7
 apple_a8
 apple_a9
 apple_latest
 apple_m1
 apple_m2
 apple_s4
 apple_s5
 carmel
 cortex_a34
 cortex_a35
 cortex_a510
 cortex_a53
 cortex_a55
 cortex_a57
 cortex_a65
 cortex_a65ae
 cortex_a710
 cortex_a715
 cortex_a72
 cortex_a73
 cortex_a75
 cortex_a76
 cortex_a76ae
 cortex_a77
 cortex_a78
 cortex_a78c
 cortex_r82
 cortex_x1
 cortex_x1c
 cortex_x2
 cortex_x3
 cyclone
 emag
 exynos_m1
 exynos_m2
 exynos_m3
 exynos_m4
 exynos_m5
 falkor
 generic
 kryo
 neoverse_512tvb
 neoverse_e1
 neoverse_n1
 neoverse_n2
 neoverse_v1
 neoverse_v2
 saphira
 thunderx
 thunderx2t99
 thunderx3t110
 thunderxt81
 thunderxt83
 thunderxt88
 tsv110
 xgene1

error: unknown CPU: 'armv8'
alexrp commented 1 month ago

Putting on my Zig MSBuild SDK hat for a moment: I find it incredibly valuable that I can use the same -target syntax for all Zig commands. This will be even more true if #20690 is accepted, as Zig's -target will then be far superior to the myriad of target/CPU/ABI options you have to deal with when using a normal C compiler.

FWIW, I'm also personally not convinced that enough build scripts in the wild actually have hardcoded -march/-mcpu/-mtune options to justify this work, or to justify making zig cc -target more obscure (-zig-target or something). Compatibility with Clang's -target option also has rather questionable value in practice considering GCC doesn't speak it.

I think we should stick to our guns here and insist that Zig's -target is, in fact, the better way forward in the long term.

andrewrk commented 1 day ago

Agreeing with @alexrp and rejecting this proposal.