Revert "nvptx: use simd_fmin and simd_fmax for minnum and maxnum"#2054
Revert "nvptx: use simd_fmin and simd_fmax for minnum and maxnum"#2054RalfJung wants to merge 1 commit intorust-lang:mainfrom
simd_fmin and simd_fmax for minnum and maxnum"#2054Conversation
…um`" This reverts commit eb1b14b.
| #[link_name = "llvm.minnum.v2f16"] | ||
| fn llvm_f16x2_minnum(a: f16x2, b: f16x2) -> f16x2; | ||
| #[link_name = "llvm.minimum.v2f16"] | ||
| fn llvm_f16x2_minimum(a: f16x2, b: f16x2) -> f16x2; |
There was a problem hiding this comment.
Hm, looking at this, this seems to still invoke the same LLVM intrinsic, not something nvptx-specific? That doesn't seem right...
There was a problem hiding this comment.
Looking at the nvptx docs, they don't mention signaling NaN but if we assume they treat all NaN the same then using llvm.minnum is wrong because LLVM can (and will) fold minnum(SNaN, x) to QNaN whereas the docs say that must return x.
They also don't mention singed zero handling, but the instruction is specified as (a < b) ? a : b which would mean that if both inputs are zero then the 2nd input is returned. None of the variants of min/max in LLVM makes that guarantee (i.e., llvm.minimum is also not a correct implementation for f16x2_min_nan).
|
I'll close the PR as this doesn't actually change which LLVM intrinsic is used, it's |
2 participants
Same as #2052, but for nvptx.
This leaves only s390x as remaining user of simd_fmin/max in this repo:
https://github.com/rust-lang/stdarch//blob/72f7d39ae9215e862ebe6f14c88b2e17de2599e7/crates/core_arch/src/s390x/vector.rs#L830-L831
This is in a
test_impl!macro which makes it sound like it's only used for tests, but the macro doesn't look like it is test-only, so I am not sure. Already the initial implementation in https://github.com/rust-lang/stdarch//commit/d5a389f1dd6369cd87b36bac887a5c7a27a28b4c used simd_fmin/max so I am not sure what the LLVM intrinsics for this would even be.r? @folkertdev