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

CPU features are not passed to clang when assembling #10411

Open
silversquirl opened this issue Dec 26, 2021 · 6 comments
Open

CPU features are not passed to clang when assembling #10411

silversquirl opened this issue Dec 26, 2021 · 6 comments
Labels
bug Observed behavior contradicts documented or intended behavior enhancement Solving this issue will likely involve adding new logic or components to the codebase. upstream An issue with a third party project that Zig uses.
Milestone

Comments

@silversquirl
Copy link
Contributor

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
@silversquirl silversquirl added the bug Observed behavior contradicts documented or intended behavior label Dec 26, 2021
@silversquirl silversquirl changed the title CPU flags are not passed to clang CPU features are not passed to clang Dec 26, 2021
@akovaski
Copy link

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)

@andrewrk andrewrk added enhancement Solving this issue will likely involve adding new logic or components to the codebase. zig cc Zig as a drop-in C compiler feature arch-arm 32-bit ARM labels Dec 27, 2021
@andrewrk andrewrk added this to the 0.10.0 milestone Dec 27, 2021
@andrewrk andrewrk added the contributor friendly This issue is limited in scope and/or knowledge of Zig internals. label Dec 27, 2021
@zheng
Copy link

zheng commented Jan 4, 2022

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

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

@richarddd
Copy link

Can we expect this in 0.11.1 or 0.12? :)

@andrewrk andrewrk modified the milestones: 0.11.1, 0.12.0, 0.13.0 Jan 29, 2024
ghuls added a commit to aertslab/scatac_fragment_tools that referenced this issue Feb 5, 2024
Don't build for Windows and some Linux versions:
  - Building HTSlib on Windows is not possible.
  - Building HTSlib does not work on Linux:
      - x86: len() as u64 is not valid on 32-bit
      - aarch64 and ppc64le:
          - expected `*mut u8`, found `&mut i8`
          - Fixed by: rust-bio/rust-htslib#415
  - Building zlib-ng does not work for all Linux targets:
      - armv7:
          - Might get fixed in the future if zig gets fixed:
            ziglang/zig#10411
ghuls added a commit to aertslab/scatac_fragment_tools that referenced this issue Feb 5, 2024
Don't build for Windows and some Linux versions:
  - Building HTSlib on Windows is not possible.
  - Building HTSlib does not work on Linux:
      - x86:
          - len() as u64 is not valid on 32-bit
      - aarch64 and ppc64le:
          - expected `*mut u8`, found `&mut i8`
          - Fixed by: rust-bio/rust-htslib#415
  - Building zlib-ng does not work for all Linux targets:
      - armv7:
          - Might get fixed in the future if zig gets fixed:
            ziglang/zig#10411
      - s390:
          - Error: Unrecognized opcode: `stfle'
@alexrp
Copy link
Contributor

alexrp commented Jul 3, 2024

Started a discussion at llvm/llvm-project#97517 to move this forward.

@andrewrk
Copy link
Member

andrewrk commented Aug 22, 2024

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.

@andrewrk andrewrk changed the title CPU features are not passed to clang CPU features are not passed to clang when assembling Aug 22, 2024
@andrewrk andrewrk added upstream An issue with a third party project that Zig uses. and removed contributor friendly This issue is limited in scope and/or knowledge of Zig internals. arch-arm 32-bit ARM labels Aug 22, 2024
@andrewrk andrewrk modified the milestones: 0.14.0, unplanned Aug 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Observed behavior contradicts documented or intended behavior enhancement Solving this issue will likely involve adding new logic or components to the codebase. upstream An issue with a third party project that Zig uses.
Projects
None yet
Development

No branches or pull requests

7 participants