Skip to content

Conversation

spavloff
Copy link
Collaborator

Increment and decrement are equivalent to adding or subtracting 1. For the floating-point values these operations depend on the current rounding mode. Teach constant evaluator to perform ++ and -- according to the current floating-point environment.

Increment and decrement are equivalent to adding or subtracting 1. For
the floating-point values these operations depend on the current
rounding mode. Teach constant evaluator to perform ++ and -- according
to the current floating-point environment.
@spavloff spavloff self-assigned this Nov 29, 2023
@llvmbot llvmbot added clang Clang issues not falling into any other category clang:frontend Language frontend issues, e.g. anything involving "Sema" labels Nov 29, 2023
@llvmbot
Copy link
Member

llvmbot commented Nov 29, 2023

@llvm/pr-subscribers-clang

Author: Serge Pavlov (spavloff)

Changes

Increment and decrement are equivalent to adding or subtracting 1. For the floating-point values these operations depend on the current rounding mode. Teach constant evaluator to perform ++ and -- according to the current floating-point environment.


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

2 Files Affected:

  • (modified) clang/lib/AST/ExprConstant.cpp (+4-2)
  • (modified) clang/test/SemaCXX/rounding-math.cpp (+21)
diff --git a/clang/lib/AST/ExprConstant.cpp b/clang/lib/AST/ExprConstant.cpp
index 16697e5f076a8f8..e0abe832c47a9f1 100644
--- a/clang/lib/AST/ExprConstant.cpp
+++ b/clang/lib/AST/ExprConstant.cpp
@@ -4623,10 +4623,12 @@ struct IncDecSubobjectHandler {
     if (Old) *Old = APValue(Value);
 
     APFloat One(Value.getSemantics(), 1);
+    llvm::RoundingMode RM =
+        E->getFPFeaturesInEffect(Info.Ctx.getLangOpts()).getRoundingMode();
     if (AccessKind == AK_Increment)
-      Value.add(One, APFloat::rmNearestTiesToEven);
+      Value.add(One, RM);
     else
-      Value.subtract(One, APFloat::rmNearestTiesToEven);
+      Value.subtract(One, RM);
     return true;
   }
   bool foundPointer(APValue &Subobj, QualType SubobjType) {
diff --git a/clang/test/SemaCXX/rounding-math.cpp b/clang/test/SemaCXX/rounding-math.cpp
index 73f9f10e6d59417..e7ead05041b560d 100644
--- a/clang/test/SemaCXX/rounding-math.cpp
+++ b/clang/test/SemaCXX/rounding-math.cpp
@@ -77,3 +77,24 @@ struct S1d {
   int f;
 };
 static_assert(sizeof(S1d) == sizeof(int), "");
+
+constexpr float incr_down(float k) {
+  float x = k;
+  ++x;
+  return x;
+}
+
+// 0x1.0p23 = 8388608.0, inc(8388608.0) = 8388609.0
+static_assert(incr_down(0x1.0p23F) == 0x1.000002p23F, "");
+// 0x1.0p24 = 16777216.0, inc(16777216.0) = 16777217.0 -> round down -> 16777216.0
+static_assert(incr_down(0x1.0p24F) == 0x1.0p24F, "");
+
+#pragma STDC FENV_ROUND FE_UPWARD
+constexpr float incr_up(float k) {
+  float x = k;
+  ++x;
+  return x;
+}
+static_assert(incr_up(0x1.0p23F) == 0x1.000002p23F, "");
+// 0x1.0p24 = 16777216.0, inc(16777216.0) = 16777217.0 -> round up -> 16777218.0
+static_assert(incr_up(0x1.0p24F) == 0x1.000002p24F, "");

@@ -4623,10 +4623,12 @@ struct IncDecSubobjectHandler {
if (Old) *Old = APValue(Value);

APFloat One(Value.getSemantics(), 1);
llvm::RoundingMode RM =
E->getFPFeaturesInEffect(Info.Ctx.getLangOpts()).getRoundingMode();
Copy link
Contributor

Choose a reason for hiding this comment

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

Shouldn't this use getActiveRoundingMode()?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes, it should. Thank you!

Copy link
Contributor

@tbaederr tbaederr left a comment

Choose a reason for hiding this comment

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

LGTM

@spavloff spavloff merged commit e620035 into llvm:main Nov 30, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clang:frontend Language frontend issues, e.g. anything involving "Sema" clang Clang issues not falling into any other category
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants