rust-lang / rust-lang/stdarch

s390x: incorrect use of LLVM intrinsics for vec_min/max on floats

Open
#2,060 13 comments 0 reactions 0 assignees View on GitHub

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

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. 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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.