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

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

Closed
andrewrk opened this issue Apr 2, 2020 · 11 comments
Closed
Labels
proposal This issue suggests modifications. If it also has the "accepted" label then it is planned. zig cc Zig as a drop-in C compiler feature
Milestone

Comments

@andrewrk
Copy link
Member

andrewrk commented Apr 2, 2020

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.

@andrewrk andrewrk added proposal This issue suggests modifications. If it also has the "accepted" label then it is planned. zig cc Zig as a drop-in C compiler feature labels Apr 2, 2020
@andrewrk andrewrk added this to the 0.7.0 milestone Apr 2, 2020
@andrewrk andrewrk modified the milestones: 0.7.0, 0.8.0 Oct 27, 2020
@andrewrk andrewrk modified the milestones: 0.8.0, 0.9.0 May 19, 2021
@mikdusan
Copy link
Member

mikdusan commented Jun 14, 2021

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:

  • restore -target, -mcpu, -march, -mtune to clang syntax/values; we'll still need to intercept and map to zig
  • add -zig-target, -zig-mcpu, -zig-march to accept zig syntax/values

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
Copy link
Contributor

jwhear commented Jun 15, 2021

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
Copy link
Contributor

jwhear commented Jun 29, 2021

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
Copy link

orent commented Sep 8, 2022

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
Copy link
Member Author

andrewrk commented Sep 8, 2022

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

@orent
Copy link

orent commented Sep 8, 2022

Was this fixed since 0.9.1?

@anagram3k
Copy link

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
Copy link

Are there any updates on this?

@andrewrk andrewrk modified the milestones: 0.11.0, 0.12.0 Jul 20, 2023
mattbryant-asana added a commit to Asana/cc-rs that referenced this issue Mar 15, 2024
This fork provides a temporary fix for ziglang/zig#4911,
by making our current Rust builds not generate targets that `zig cc`
doesn't understand.
mattbryant-asana added a commit to Asana/cc-rs that referenced this issue Mar 15, 2024
This fork provides a temporary fix for ziglang/zig#4911,
by making our current Rust builds not generate targets that `zig cc`
doesn't understand.
rvolosatovs added a commit to rvolosatovs/nixify that referenced this issue Mar 25, 2024
reorder args to ensure `target` is correctly overriden for compatibility
with zig

ziglang/zig#4911

Signed-off-by: Roman Volosatovs <rvolosatovs@riseup.net>
rvolosatovs added a commit to rvolosatovs/nixify that referenced this issue Mar 25, 2024
reorder args to ensure `target` is correctly overriden for compatibility
with zig

ziglang/zig#4911

Signed-off-by: Roman Volosatovs <rvolosatovs@riseup.net>
mattbryant-asana added a commit to Asana/cc-rs that referenced this issue Mar 28, 2024
This fork provides a temporary fix for ziglang/zig#4911,
by making our current Rust builds not generate targets that `zig cc`
doesn't understand.
@breezewish
Copy link

breezewish commented Apr 22, 2024

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
Copy link
Member

alexrp commented Jul 26, 2024

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 andrewrk modified the milestones: 0.14.0, 0.16.0 Aug 8, 2024
mattbryant-asana added a commit to Asana/cc-rs that referenced this issue Aug 22, 2024
This fork provides a temporary fix for ziglang/zig#4911,
by making our current Rust builds not generate targets that `zig cc`
doesn't understand.
@andrewrk
Copy link
Member Author

Agreeing with @alexrp and rejecting this proposal.

@andrewrk andrewrk closed this as not planned Won't fix, can't repro, duplicate, stale Aug 23, 2024
@andrewrk andrewrk modified the milestones: 0.16.0, 0.14.0 Aug 24, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
proposal This issue suggests modifications. If it also has the "accepted" label then it is planned. zig cc Zig as a drop-in C compiler feature
Projects
None yet
Development

No branches or pull requests

8 participants