-
-
Notifications
You must be signed in to change notification settings - Fork 2.8k
Fix soft float support, split musl triples by float ABI, and enable CI #21269
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
Conversation
@@ -1442,19 +1442,22 @@ test "store vector with memset" { | |||
if (builtin.zig_backend == .stage2_riscv64) return error.SkipZigTest; | |||
|
|||
if (builtin.zig_backend == .stage2_llvm) { | |||
// LLVM 16 ERROR: "Converting bits to bytes lost precision" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note that this test is being re-enabled for all targets in #21183.
Just to give an idea of the direction I'm working towards, the next PR will include:
|
Seems like the CI impact here is within reason. |
…nux-musleabi(hf)`. Closes ziglang#21184.
Also rename Target.getFloatAbi() to floatAbi().
Prefer `eabi` and `eabihf` over the ambiguous `none` when not using libc.
Broadly speaking, versions 6, 7, and 8 are the ones that are in common use. Of these, v7 is what you'll typically see for 32-bit Arm today. So let's actually make sure that that's what we're testing.
Similar to __truncsfhf2() and __extendhfsf2().
// `use-soft-float` means "use software routines for floating point computations". In | ||
// other words, it configures how LLVM lowers basic float instructions like `fcmp`, | ||
// `fadd`, etc. The float calling convention is configured on `TargetMachine` and is | ||
// mostly an orthogonal concept, although obviously we do need hardware float operations | ||
// to actually be able to pass float values in float registers. | ||
// | ||
// Ideally, we would support something akin to the `-mfloat-abi=softfp` option that GCC | ||
// and Clang support for Arm32 and CSKY. We don't currently expose such an option in | ||
// Zig, and using CPU features as the source of truth for this makes for a miserable | ||
// user experience since people expect e.g. `arm-linux-gnueabi` to mean full soft float | ||
// unless the compiler has explicitly been told otherwise. (And note that our baseline | ||
// CPU models almost all include FPU features!) | ||
// | ||
// Revisit this at some point. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What makes sense to me is:
- float ABI in the target triple
- whether hard float instructions can be used in the CPU featureset
Furthermore, baseline for soft float ABI target triples can default to not using float instructions in the CPU features.
it's unclear to me if this comment is in agreement with this or not.
Thanks for working on this. So far I'm unconvinced that Zig needs another option for controlling the ABI or CPU features. Based on your description it seems to me like this use case can be fully addressed by adjusting the baseline CPU featureset that is selected for soft float ABI targets. |
Fix soft float support, split musl triples by float ABI, and enable CI
This is the first round of soft float work that I'm doing.
There are 3 goals of this PR:
I'm completely ignoring
-mfloat-abi=softfp
support (soft float ABI with FPU computations) and in fact intentionally suppressing it in the LLVM backend as a result of theuse-soft-float
change. The only thing that matters for this first patch set is correctness; the next round of patches will re-enable support for this optimization in a way that actually functions correctly.Note that a core premise of my work in this area is that, if the user specifies a soft float target triple, the default assumption should be that no FPU code is generated, even if the target CPU model happens to have FPU features. There's a very strong established precedent for this in the C/C++ world where you have to explicitly opt into this behavior by using
-mfloat-abi=softfp
instead of-mfloat-abi=soft
(the latter being implied by the soft float triple). But even without that, everyone understands e.g.arm-linux-gnueabi
to mean soft float, and it's just astonishing as a user if the resulting binary fromzig build -Dtarget=arm-linux-gnueabi
has FPU instructions in it and can't run on a fully soft float system.I initially thought that adding soft float targets to CI wasn't necessary, but the state of our soft float support was broken enough that I've changed my mind on this, so this PR also includes those test additions. Let's see if it causes a noticeable slowdown, and if so, we can maybe limit it to Arm or something. (As usual, the MIPS O32 targets are gated by
-Dtest-slow-targets
which is off by default both locally and in CI.) This branch passeszig build test -fqemu -fwasmtime --glibc-runtimes <...> -Dtest-slow-targets
locally for me with all the added targets. Finally, I also changed the Arm target triples for tests to use v7 instead of v8 because a) v7 is much more common for 32-bit Arm, and b) v8 adds floating point features that don't exist in v7 so things were broken for v7 and it went unnoticed.Closes #10961.
Closes #21184.