s390x: incorrect use of LLVM intrinsics for vec_min/max on floats
Nobody has claimed this yet.
- Dominant language
- Rust
- Stars
- 694
- Forks
- 336
- Avg merge
- 5d 14h
- Merged PRs (30d)
- 9
Description
The s390x vec_min/vec_max intrinsics on floats are meant to behave like the IEEE 754-2008 minNum/maxNum operation (with -0.0 < +0.0). However, we are currently using the llvm.minnum/llvm.maxnum intrinsic for them, which has different behavior for SNaN: LLVM is allowed to fold minnum(SNaN, x) into x but the intrinsic is supposed to always return a NaN for that situation. (I don't know if LLVM will actually ever use this freedom, but it is in the spec so who knows what it does in the future.)
We should be using llvm.s390.vfminsb and friends for this (as in https://github.com/rust-lang/stdarch/commit/6efd06e975e7dea1f5b7e1f1db5aba51237feaca), but that also does not work because the underlying hardware instruction is only available when the vector-enhancements-1 target feature is enabled, and whole llvm.minnum has a fallback that gets used when that target feature is missing, llvm.s390.* does not.
At the moment, I don't think there is a good way to fix this: the odd behavior of vec_min having to be available even when the underlying instruction is not available means that a fallback is needed, and that can only really be implemented in LLVM (or with a special hack in rustc codegen), but LLVM has no portable intrinsic that has the right SNaN behavior and the arch-specific intrinsic does not have the fallback behavior we need. I wonder what Clang does. (EDIT: see here for what clang does.)
Cc @uweigand
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start at the s390x vec_min/vec_max intrinsic entry points and inspect their use of llvm.minnum/llvm.maxnum versus llvm.s390.vfminsb and related intrinsics. Compare the behavior with Clang as referenced in the issue. Done requires preserving the required SNaN and signed-zero semantics while retaining a fallback when vector-enhancements-1 is unavailable.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- rust
- Domain
- compilers
- Issue type
- Bug
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Stale
- Clarity
- Needs clarification
- Newbie friendliness
- 25/100