ziglang / zig

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

CPU features are not passed to clang when assembling #10411

Open silversquirl opened 2 years ago

silversquirl commented 2 years ago

Zig Version

0.10.0-dev.40+303bad998

Steps to Reproduce

Create an ARM assembly file named test.s with the following contents:

mov r1, #0

Compile that file to an object using zig build-obj --verbose-cc -target arm-freestanding -mcpu cortex_m0plus+thumb2 test.s

Expected Behavior

Zig passes the +thumb2 option to clang, which correctly assembles the thumb2 instruction (though this particular CPU feature won't work until llvm/llvm-project#52878 is fixed).

Actual Behavior

The +thumb2 feature is not passed to zig clang:

$ zig build-obj --verbose-cc -target arm-freestanding -mcpu cortex_m0plus+thumb2 test.s
/home/silver/.opt/zig/zig clang -fno-caret-diagnostics -target arm-unknown-unknown-eabi -mcpu=cortex-m0plus -ffreestanding -c -o /home/silver/.cache/zig/tmp/c60723f3c667529f-test.o test.s
error(compilation): clang failed with stderr: zig: warning: argument unused during compilation: '-fno-caret-diagnostics' [-Wunused-command-line-argument]
zig: warning: argument unused during compilation: '-ffreestanding' [-Wunused-command-line-argument]
test.s:1:1: error: invalid instruction, any one of the following would fix this:
mov r1, #0
^
test.s:1:9: note: operand must be a register in range [r0, r15]
mov r1, #0
        ^
test.s:1:1: note: instruction requires: thumb2
mov r1, #0
^

test.s:1:1: error: unable to build C object: clang exited with code 1
akovaski commented 2 years ago

Just some info: This is already somewhat known (Not that there's a duplicate issue on github), see the .assembly part of addCCArgs in Compilation.zig:

// The Clang assembler does not accept the list of CPU features like the
// compiler frontend does. Therefore we must hard-code the -m flags for
// all CPU features here.

I'm not sure what exactly this comment is referring to, but I suspect it is due to clang's -Xassembler argument not passing -target-feature to the assembler [1] [2]. (Zig uses -Xclang -target-feature -Xclang -<feature> to pass target features for C files.)

So manually adding exceptions is the current state. I expect Zig could pass -target-feature if calling the Clang assembly compiler directly with the -cc1as flag (.e.g clang-13 -cc1as -triple riscv64 ./os/entry.s -target-feature -relax). You'd also have to adjust how Zig passes all the other flags to Clang for assembly files, because clang -cc1as has different flags from clang.

(P.S. There's also a corresponding -cc1 flag for Clang's C compiler)

zheng commented 2 years ago

Just some info: This is already somewhat known (Not that there's a duplicate issue on github), see the .assembly part of addCCArgs in Compilation.zig:

// The Clang assembler does not accept the list of CPU features like the
// compiler frontend does. Therefore we must hard-code the -m flags for
// all CPU features here.

I'm not sure what exactly this comment is referring to, but I suspect it is due to clang's -Xassembler argument not passing -target-feature to the assembler [1] [2]. (Zig uses -Xclang -target-feature -Xclang -<feature> to pass target features for C files.)

So manually adding exceptions is the current state. I expect Zig could pass -target-feature if calling the Clang assembly compiler directly with the -cc1as flag (.e.g clang-13 -cc1as -triple riscv64 ./os/entry.s -target-feature -relax). You'd also have to adjust how Zig passes all the other flags to Clang for assembly files, because clang -cc1as has different flags from clang.

(P.S. There's also a corresponding -cc1 flag for Clang's C compiler)

Very helpful - thanks for all the context!

I was able to run the clang compiler frontend and pass in the target feature in the example in the format you described. I'm going to try and get the various assembly flags right and put up a PR.

mikdusan commented 1 year ago

found this stalled patch from '2014, though we'd need a few more passthru-args enabled: https://reviews.llvm.org/D6170

richarddd commented 10 months ago

Can we expect this in 0.11.1 or 0.12? :)

alexrp commented 4 months ago

Started a discussion at https://github.com/llvm/llvm-project/issues/97517 to move this forward.

andrewrk commented 3 months ago

There is a clean long-term solution to this problem that does not rely on any changes from LLVM:

This is already planned. My suggestion is to put effort into the long-term solution rather than the short-term one, especially now that the upstream issue has been closed by LLVM maintainers who have no interest in fixing this problem.

alexrp commented 1 month ago

Just FYI for anyone affected by this: https://github.com/ziglang/zig/pull/21574#issuecomment-2390665665