copysign with sign being a NaN can have non-portable results

[RalfJung]

Follow-up to #129559.
Cc @tgross35 @beetrees

There's no portable variant we can recommend instead here, is there? Something with a semantics like "if sign is a NaN, then return self unaltered, otherwise return self with the sign changed to that of sign"?

[rustbot]

r? @workingjubilee

rustbot has assigned @workingjubilee.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

[RalfJung]

r? @thomcc
since you hopefully still have context from #129559

[bjorn3]

Doesn't copysign operate on bit level rather than as an arithmetic operation? On riscv copysign would be implemented using fsgnj which is documented as working with individual bits and not canonicalizing NaN. On x86 LLVM uses two andps and an orps which works with individual bits too. On arm64 LLVM uses bif, which too works on individual bits. For C++ https://en.cppreference.com/w/cpp/numeric/math/copysign documents copysign as the only portable way of manipulating the sign of NaN.

[RalfJung]

Yes, it does work on the bit level. That is already documented:

If self is a NaN, then a NaN with the same payload as self and the sign bit of sign is returned.

[RalfJung]

The point isn't that copysign itself behaves different on different targets, the point is that it can make the sign of a NaN visible and that can be different between different targets. If a floating-point program never uses is_sign_*, copysign, or the conversion to raw bytes, it will behave exactly the same on all platforms -- that's what makes those operations special and why they have some warnings and links in their docs.

[workingjubilee]

The NaN bit pattern docs don't mention what operations are arithmetic operations.

[thomcc]

LGTM
@bors r+ rollup

[bors]

📌 Commit 0589dc7 has been approved by thomcc

It is now in the queue for this repository.

[workingjubilee]

The following should be consistent, right, even if it isn't specified what the values are?

let nan = f32::NAN;
let sign_a = 1.0_f32.copysign(nan);
let sign_b = 1.0_f32.copysign(f32::from_bits(nan.to_bits()));
assert_eq!(sign_a, sign_b);

[RalfJung]

@workingjubilee yes, from_bits(f.to_bits()) is a perfect round-trip.

[workingjubilee]

Then we should document that some operations are non-arithmetic operations and what they are.

[RalfJung]

The NaN bit pattern docs don't mention what operations are arithmetic operations.

The NaN rules apply to all float operations except the "bitwise" ones, unless the operation documents something different. The docs do say which ones are "bitwise".

So the use of "arithmetic" in these docs here isn't very meaningful I think -- but I didn't introduce that term into the docs so I am not sure what the original author intended that term to mean.

[workingjubilee]

I suspect they meant the set that doesn't include abs, copysign, signum, *_bits, and is_sign_*.

[workingjubilee]

That's what IEEE754 would say, anyways.

I bring this up because that's the precise set we're declaring as having inconstant results, essentially.

[RalfJung]

That comment applies more to #129559 than to this PR. I opened #129699 to not lose track.

[workingjubilee]

Sure!

I am mostly making sure that there's no objection to clarifying this matter further, because I know that even if we "get it", I don't think it will be understood implicitly by everyone.