Skip to content
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

rint and nearbyint intrinsic documentation is misleading about exceptions #77561

Open
RalfJung opened this issue Jan 10, 2024 · 8 comments
Open
Labels
documentation floating-point Floating-point math llvm Umbrella label for LLVM issues

Comments

@RalfJung
Copy link
Contributor

The rint intrinsic makes this statement about FP exceptions:

It may raise an inexact floating-point exception if the operand isn’t an integer.

nearbyint differs from rint by not having that statement.

However, this is misleading, since LLVM also documents "the default LLVM floating-point environment assumes that traps are disabled and status flags are not observable". This means that whether or not rint raises an exception is assumed to be un-observable, and LLVM will freely reorder rint with other operations that might alter FP exception flags. Effectively, rint and nearbyint provide equivalent semantics (as least that's how LLVM frontends should treat the situation), and anyone relying on rint doing anything with exceptions will be disappointed eventually.

It would probably be better to remove any mention of exceptions in the rint and nearbyint docs, and possibly to merge those intrinsics. People relying on exception behavior need to use the constrained intrinsics.

@RalfJung
Copy link
Contributor Author

RalfJung commented Jan 10, 2024 via email

@dtcxzyw
Copy link
Member

dtcxzyw commented Jan 10, 2024

Which PR closed this issue?

Didn't it resolved by #77191?

@RalfJung
Copy link
Contributor Author

RalfJung commented Jan 10, 2024 via email

@dtcxzyw
Copy link
Member

dtcxzyw commented Jan 10, 2024

Oh, sorry for my misreading.

@dtcxzyw dtcxzyw reopened this Jan 10, 2024
@dtcxzyw dtcxzyw added floating-point Floating-point math llvm Umbrella label for LLVM issues and removed llvm Umbrella label for LLVM issues labels Jan 10, 2024
@arsenm
Copy link
Contributor

arsenm commented Jan 11, 2024

This is an unfortunate leftover from before there was a design for strictfp support. We now have 3 intrinsic that mean the same thing. nearbyint, rint and roundeven all do the same thing. We should probably drop llvm.nearbyint and llvm.rint, and auto upgrade them to roundeven (but this would potentially not be worth the asymmetry with the constrained intrinsics)

@RalfJung
Copy link
Contributor Author

That still seems preferable over the confusion caused when frontends think there is a meaningful difference between these intrinsics.

@RalfJung
Copy link
Contributor Author

RalfJung commented Jan 11, 2024

That said, when Rust tried to use roundeven we ran into issues since it is less widely supported by platforms' libm than the alternatives.

@arsenm
Copy link
Contributor

arsenm commented Jan 12, 2024

That said, when Rust tried to use roundeven we ran into issues since it is less widely supported by platforms' libm than the alternatives.

That's a lowering issue. We should just start remapping the calls to whatever call exists instead of expecting exact match like happens now

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation floating-point Floating-point math llvm Umbrella label for LLVM issues
Projects
None yet
Development

No branches or pull requests

4 participants