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

LangRef: rint, nearbyint: mention that default rounding mode is assumed #77191

Merged
merged 1 commit into from
Jan 9, 2024

Conversation

RalfJung
Copy link
Contributor

@RalfJung RalfJung commented Jan 6, 2024

According to the docs, LLVM assumes round-to-nearest mode. The source code also contains some indications of LLVM constant-folding these intrinsics without regard for a possibly changed rounding mode:

// Non-constrained variants of rounding operations means default FP
// environment, they can be folded in any case.
case Intrinsic::ceil:
case Intrinsic::floor:
case Intrinsic::round:
case Intrinsic::roundeven:
case Intrinsic::trunc:
case Intrinsic::nearbyint:
case Intrinsic::rint:
case Intrinsic::canonicalize:

if (IntrinsicID == Intrinsic::nearbyint || IntrinsicID == Intrinsic::rint) {
U.roundToIntegral(APFloat::rmNearestTiesToEven);
return ConstantFP::get(Ty->getContext(), U);
}

@llvmbot llvmbot added the llvm:ir label Jan 6, 2024
@llvmbot
Copy link
Member

llvmbot commented Jan 6, 2024

@llvm/pr-subscribers-llvm-ir

Author: Ralf Jung (RalfJung)

Changes

According to the docs, LLVM assumes round-to-nearest mode. The source code also contains some indications of LLVM constant-folding these intrinsics without regard for a possibly changed rounding mode:

// Non-constrained variants of rounding operations means default FP
// environment, they can be folded in any case.
case Intrinsic::ceil:
case Intrinsic::floor:
case Intrinsic::round:
case Intrinsic::roundeven:
case Intrinsic::trunc:
case Intrinsic::nearbyint:
case Intrinsic::rint:
case Intrinsic::canonicalize:

if (IntrinsicID == Intrinsic::nearbyint || IntrinsicID == Intrinsic::rint) {
U.roundToIntegral(APFloat::rmNearestTiesToEven);
return ConstantFP::get(Ty->getContext(), U);
}


Full diff: https://github.com/llvm/llvm-project/pull/77191.diff

1 Files Affected:

  • (modified) llvm/docs/LangRef.rst (+6-2)
diff --git a/llvm/docs/LangRef.rst b/llvm/docs/LangRef.rst
index b5918e3063d868..772620a3630b3e 100644
--- a/llvm/docs/LangRef.rst
+++ b/llvm/docs/LangRef.rst
@@ -15751,7 +15751,9 @@ Semantics:
 """"""""""
 
 This function returns the same values as the libm ``rint`` functions
-would, and handles error conditions in the same way.
+would, and handles error conditions in the same way. The rounding mode
+is assumed to be set to "nearest", so halfway cases are rounded to the
+even integer.
 
 .. _int_nearbyint:
 
@@ -15789,7 +15791,9 @@ Semantics:
 """"""""""
 
 This function returns the same values as the libm ``nearbyint``
-functions would, and handles error conditions in the same way.
+functions would, and handles error conditions in the same way. The
+rounding mode is assumed to be set to "nearest", so halfway cases
+are rounded to the even integer.
 
 .. _int_round:
 

@@ -15751,7 +15751,9 @@ Semantics:
""""""""""

This function returns the same values as the libm ``rint`` functions
would, and handles error conditions in the same way.
would, and handles error conditions in the same way. The rounding mode
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What do you intend by the "handles error conditions in the same way" part of this? The C documentation for rint is a bit ambiguous in that it indicates that the function "may" raise FE_INEXACT, but it doesn't require it or say what will be done about it. Near as I can tell, the function doesn't actually perform any erorr handling. It just allows the implementation to raise the exception.

The fact that we have both llvm.rint and llvm.nearbyint is a bit of a problem, I think. Since "the default LLVM floating-point environment assumes that traps are disabled and status flags are not observable" (per the language reference), there should be no difference at all between llvm.rint and llvm.nearbyint.

If we intend to forbid implementations of llvm.nearbyint from raising the FE_INEXACT exception, that should be explicitly documented. On the other hand, if we aren't requiring that, we should probably say that explicitly too.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

rint corresponds to the IEEE 754 operation roundToIntegralExact, which signals inexact if the rounding isn't exact (i.e., most of the time). nearbyint corresponds to a theoretical operation roundToIntegral that never signals inexact, but this operation doesn't exist in IEEE 754 [1]; the variants that do exist all effectively use standard rounding modes.

[1] It was apparently a recommended operation in IEEE 854, but the committee appears to have explicitly voted to not make it a required operation. See https://speleotrove.com/decimal/854M8507.pdf for more details.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a version of the C standard that specifies that rint will raise the inexact exception if the result isn't exact? The version I looked at (a draft version of C11), says it "may" raise the inexact exception.

Either way, I think this is irrelevant for the LLVM intrinsic since "The default LLVM floating-point environment assumes that traps are disabled and status flags are not observable."

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What do you intend by the "handles error conditions in the same way" part of this?

I didn't write this, it is existing text. I don't know or understand the intent of it.

The fact that we have both llvm.rint and llvm.nearbyint is a bit of a problem, I think. Since "the default LLVM floating-point environment assumes that traps are disabled and status flags are not observable" (per the language reference), there should be no difference at all between llvm.rint and llvm.nearbyint.

Agreed. However, removing one of them seemed like a bigger change than I can muster, so I decided to focus on rounding only and leave the existing text unchanged for exceptions.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

n3149 which passed the C23 DIS ballot states that rint functions may raise the "inexact" floating-point exception

7.12.9.4 The rint functions
Synopsis
1

#include <math.h>
double rint(double x);
float rintf(float x);
long double rintl(long double x);
#ifdef __STDC_IEC_60559_DFP__
_Decimal32 rintd32(_Decimal32 x);
_Decimal64 rintd64(_Decimal64 x);
_Decimal128 rintd128(_Decimal128 x);
#endif

Description
2 The rint functions differ from the nearbyint functions (7.12.9.3) only in that the rint functions
may raise the "inexact" floating-point exception if the result differs in value from the argument.

Returns
3 The rint functions return the rounded integer value.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This isn't how they're handled today, but I have a model of how to support FP exceptions better in LLVM IR, which draws from C's FENV_ACCESS rules. In non-exception-aware code, the state of the FP exception bits would be unspecified (probably poison) at any point in time, whereas exception-aware code would have the bits set as specified by IEEE 754. This model would largely reuse the existing intrinsics for operations (and with a few other changes I think could eliminate the need for duplicated constrained intrinsic versions), so I've already largely mentally mapped every such intrinsic to the corresponding IEEE 754 operation, even in cases where two operations would have the same effect but for exception handling, which technically requires the experimental constrained intrinsics at the moment.

I'm not clear how you intend to achieve such a thing, but it sounds interesting. How would we distinguish "exception-aware code" from "non-exception-aware code"?

I think the main reason for LLVM's non-handling of exceptions is that assuming instructions don't have side-effect frees us to do a lot of code motion without having to worry about side-effects. The constrained intrinsics were introduced, in part, as a way to indicate when we needed side-effects to be respected. I'd be all for eliminating the redundant intrinsics, but we'd need a way to ensure that all code which operates on FP instructions and calls would handle the case when side-effects needed to be enforced.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In any case, I think the exception behavior doesn't need to be addressed by this patch.

:)

I don't have permissions in this repo, @andykaylor since you approved -- can you also do the merge or how does that work?

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I checked with the chair of the WG14 floating-point study group and received this response:

Clause 7 is for all implementations, not just Annex F ones, so cannot require floating-point exceptions. See F.10.6.4. It’s implementation-defined behavior, but Annex F defines the behavior for implementations that support it.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I checked with the chair of the WG14 floating-point study group and received this response:

Clause 7 is for all implementations, not just Annex F ones, so cannot require floating-point exceptions. See F.10.6.4. It’s implementation-defined behavior, but Annex F defines the behavior for implementations that support it.

That makes sense, but it takes it back to the question of the LLVM default environment. Since the default environment doesn't support exceptions, the llvm.rint intrinsic shouldn't have such a requirement and front ends that intend to support full exception semantics should use the constrained intrinsic (at least until we have a scheme that handles fp exceptions in a better way).

Anyone who relies on IEEE-753/IEC 60559 exception semantics with the default instructions and intrinsics is going to be disappointed sooner or later. You end up with something that might work sometimes, maybe even most of the time, but it won't be robust enough for critical work. (See, for example, #34581)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have opened an issue to track this, since discussions in PRs are likely to be lost: #77561.

llvm/docs/LangRef.rst Outdated Show resolved Hide resolved
@RalfJung RalfJung force-pushed the langref-rounding-mode branch from 255b63e to 0251728 Compare January 9, 2024 07:49
Copy link
Contributor

@andykaylor andykaylor left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@andykaylor andykaylor merged commit fb14662 into llvm:main Jan 9, 2024
4 of 5 checks passed
@RalfJung RalfJung deleted the langref-rounding-mode branch January 9, 2024 22:06
justinfargnoli pushed a commit to justinfargnoli/llvm-project that referenced this pull request Jan 28, 2024
…ed (llvm#77191)

LLVM assumes round-to-nearest mode and sometimes performs constant-folding based on that assumption. This updates the language ref documentation for the rint and nearbyint intrinsics to mention that fact.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants