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

Clang: Support for -Xassembler -target-feature (or similar) #97517

Closed
alexrp opened this issue Jul 3, 2024 · 5 comments
Closed

Clang: Support for -Xassembler -target-feature (or similar) #97517

alexrp opened this issue Jul 3, 2024 · 5 comments
Labels
clang:driver 'clang' and 'clang++' user-facing binaries. Not 'clang-cl' question A question, not bug report. Check out https://llvm.org/docs/GettingInvolved.html instead!

Comments

@alexrp
Copy link
Member

alexrp commented Jul 3, 2024

Zig needs the ability to pass target features along to the integrated assembler - see ziglang/zig#10411.

For C files, Zig uses -Xclang -target-feature. For assembly, the only option at the moment is to switch to clang -cc1as but that would require massive surgery in Zig's driver code since it has a shared code path (with tons of logic) for both at the moment.

There is a very simple change that would solve this headache: Update Clang's CollectArgsForIntegratedAssembler() to accept -target-feature and pass it along. (Or even pass anything along, as -Xclang does.)

Alternatively, if -Xassembler has to be kept GNU-compatible only, we could imagine adding a new -Xclangas that just passes the arguments through like -Xclang.

Would either of these options be acceptable? If so, I could probably do a PR.

@github-actions github-actions bot added the clang Clang issues not falling into any other category label Jul 3, 2024
@alexrp alexrp changed the title Clang: Support for -Xassembler -target-feature Clang: Support for -Xassembler -target-feature (or similar) Jul 3, 2024
@dtcxzyw dtcxzyw added clang:driver 'clang' and 'clang++' user-facing binaries. Not 'clang-cl' and removed clang Clang issues not falling into any other category labels Jul 3, 2024
@llvmbot
Copy link
Member

llvmbot commented Jul 3, 2024

@llvm/issue-subscribers-clang-driver

Author: Alex Rønne Petersen (alexrp)

Zig needs the ability to pass target features along to the integrated assembler - see https://github.com/ziglang/zig/issues/10411.

For C files, Zig uses -Xclang -target-feature. For assembly, the only option at the moment is to switch to clang -cc1as but that would require massive surgery in Zig's driver code since it has a shared code path (with tons of logic) for both at the moment.

There is a very simple change that would solve this headache: Update Clang's CollectArgsForIntegratedAssembler() to accept -target-feature and pass it along. (Or even pass anything along, as -Xclang does.)

Alternatively, if -Xassembler has to be kept GNU-compatible only, we could imagine adding a new -Xclangas that just passes the arguments through like -Xclang.

Would either of these options be acceptable? If so, I could probably do a PR.

@alexrp
Copy link
Member Author

alexrp commented Jul 16, 2024

Friendly ping; I'm happy to do the work here, just need to know if either approach is acceptable. 🙂

@MaskRay
Copy link
Member

MaskRay commented Jul 26, 2024

Exposing an arbitrary cc1as forwarder in clangDriver is not ideal. It could lead to a surge of compatibility issues "please remove the incompatible target-feature when I specify -target-feature +X".
We cannot sign up this busy work.

let's focus on the immediate concern.

In zig build-obj --verbose-cc -target arm-freestanding -mcpu cortex_m0plus+thumb2 x.s, zig passes -mcpu=cortex-m0plus to clang. davemgreen has explained that Cortex-M0+ doesn't support the instruction, so Clang correctly rejected assembling.

@MaskRay MaskRay closed this as not planned Won't fix, can't repro, duplicate, stale Jul 26, 2024
@EugeneZelenko EugeneZelenko added the question A question, not bug report. Check out https://llvm.org/docs/GettingInvolved.html instead! label Jul 26, 2024
@alexrp
Copy link
Member Author

alexrp commented Jul 26, 2024

@MaskRay

Exposing an arbitrary cc1as forwarder in clangDriver is not ideal. It could lead to a surge of compatibility issues "please remove the incompatible target-feature when I specify -target-feature +X".
We cannot sign up this busy work.

Can you expand on what you mean by this? I put the -Xclangas processing where I did so that any arguments passed that way come after -target-feature args passed by Clang itself. The thinking would be that anyone using -Xclangas knows what they're doing and wants to override anything prior.

If it would help alleviate concerns, could we just not document -Xclangas? Give it a more obscure name? Or even limit it to just -target-abi, -target-cpu, and -target-feature to at least keep the potential for abuse somewhat under control?

The alternative for us is completely upending our Clang wrapper code and invoking clang -cc1 and clang -cc1as directly. That's undesirable for many good reasons.

let's focus on the immediate concern.

That example is missing the forest for the trees. 🙂 The general issue is that we need a way to communicate ABI, CPU model, and CPU features to the LLVM assembler. It manifests in many other ways where it actually is a legitimate problem. For example, zig cc currently can't build libffi for arm-linux-gnueabi (as opposed to arm-linux-gnueabihf) because libffi contains a blx lr instruction, which requires +v5t.

To understand why we can't reasonably solve this today, you have to understand that Zig fundamentally operates in terms of: 1) target triple (implies ABI), 2) CPU model (implies some CPU default features), 3) CPU features. The CPU model and features are extracted nearly 1:1 from LLVM, and due to the way CPU features are modeled in LLVM, there is no easy mapping back to Clang-level -march/-mcpu options. So we need some way to talk directly to LLVM to communicate these features.

You can see our current approach for dealing with this here. It's easy to see how this is not sustainable.

@alexrp
Copy link
Member Author

alexrp commented Jul 26, 2024

@MaskRay Also, note that -Xclang -target-feature -Xclang <...> is already a thing today, and we're using it. We're really just asking to have the same feature that already exists extended to assembly files.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clang:driver 'clang' and 'clang++' user-facing binaries. Not 'clang-cl' question A question, not bug report. Check out https://llvm.org/docs/GettingInvolved.html instead!
Projects
None yet
Development

No branches or pull requests

5 participants