llvm / llvm-project

The LLVM Project is a collection of modular and reusable compiler and toolchain technologies.
http://llvm.org
Other
27.91k stars 11.52k forks source link

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

Closed alexrp closed 1 month ago

alexrp commented 2 months ago

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.

llvmbot commented 2 months ago

@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()`](https://github.com/llvm/llvm-project/blob/6a992bc89f5ca25d132abd044d78ecf27ae6e162/clang/lib/Driver/ToolChains/Clang.cpp#L2464) to accept `-target-feature` and pass it along. (Or even pass *anything* along, as `-Xclang` [does](https://github.com/llvm/llvm-project/blob/6a992bc89f5ca25d132abd044d78ecf27ae6e162/clang/lib/Driver/ToolChains/Clang.cpp#L7553).) 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 commented 1 month ago

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

MaskRay commented 1 month ago

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.

alexrp commented 1 month ago

@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. :slightly_smiling_face: 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 commented 1 month ago

@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.