[FPEnv] Evaluate constant expressions under non-default rounding modes

[sepavloff]

Summary

The change implements evaluation of constant floating point expressions
under non-default rounding modes. The main objective was to support
evaluation of global variable initializers, where constant rounding mode
may be specified by #pragma STDC FENV_ROUND.

[rjmccall]

clang/lib/AST/ExprConstant.cpp

2427

I think the options really need to be passed in or else correctness is somewhat doomed here.

For example, the call to CompoundAssignSubobjectHandler needs to propagate this down from the operator expression.

2678

Feels like you should have a helper function like getActiveRoundingMode(Info, FPFeatures)

2712

Is this processing something you can extract meaningfully into a common function that works with an opStatus, so that you can just write if (!checkFloatingPointFailure(Info, E, status, fpFeatures)) return false; in all these places?

[sepavloff]

Updated patch

[sepavloff]

clang/lib/AST/ExprConstant.cpp

2427

It is guaranteed by the way AST is built, no?

As FP options may be changed only by pragmas and the pragmas can be specified only at file or block level, all sub-expression are evaluated at the same options.

2678

It required implementation of statically polymorphic method Expr::getFPFeaturesInEffect but finally code looks better.

2712

Good idea, thank you. I used a bit different name to have positive semantics.

[rjmccall]

clang/lib/AST/ExprConstant.cpp

2427

Yes, but you can't actually reliably recover those settings from E unless you're sure E is one of a few kinds of expression. The concern is that E might end up being some closely-related expression that isn't actually the expression that carries the current settings, and then we'll fall back on using the global defaults. It's much more correct-by-construction to pass the settings down from the caller based on the caller's static knowledge of which expression is under consideration, and I think you'll see that that's pretty straightforward in practice.

2462

Thanks, these look good.

[sepavloff]

Updated patch

• improved Expr::getFPFeaturesInEffect,
• added tests for _Complex,
• added tests for constexpr,
• added chech in HandleFloatToFloatCast,
• implemented printing FPOptions in CompoundAssignOperator.

[sepavloff]

clang/lib/AST/ExprConstant.cpp

2427

This is a peculiarity of CompoundAssignOperator, which itself makes conversion, without CastExpr. I added assert to ensure that other nodes cannot appear here. Also some tests with conversion in CompoundAssignOperator were added.

[rjmccall]

clang/lib/AST/ExprConstant.cpp

2427

Alright, well, I don't understand the reluctance to pass this down, but I won't insist. LGTM.

[mibintc]

clang/lib/AST/ExprConstant.cpp

2438

If the result is this false, shall this routine create a Note diag explaining the false result?

[sepavloff]

Added diagnostic message

[sepavloff]

clang/lib/AST/ExprConstant.cpp

2438

It makes sense. Added respective diagnostics.

[leonardchan]

Hi! It looks like this is causing a test failure on our aach64 builders (https://luci-milo.appspot.com/p/fuchsia/builders/ci/clang-linux-arm64/b8868095822628984976?):

[1113/1114] Running the Clang regression tests
llvm-lit: /b/s/w/ir/k/staging/llvm_build/bin/../../../llvm-project/llvm/utils/lit/lit/llvm/config.py:379: note: using clang: /b/s/w/ir/k/staging/llvm_build/bin/clang
-- Testing: 26708 tests, 256 workers --
Testing: 
FAIL: Clang :: AST/const-fpfeatures-diag.c (269 of 26708)
******************** TEST 'Clang :: AST/const-fpfeatures-diag.c' FAILED ********************
Script:
--
: 'RUN: at line 1';   /b/s/w/ir/k/staging/llvm_build/bin/clang -cc1 -internal-isystem /b/s/w/ir/k/staging/llvm_build/lib/clang/12.0.0/include -nostdsysteminc -verify -ffp-exception-behavior=strict -Wno-unknown-pragmas /b/s/w/ir/k/llvm-project/clang/test/AST/const-fpfeatures-diag.c
--
Exit Code: 1

Command Output (stderr):
--
error: 'error' diagnostics expected but not seen: 
  File /b/s/w/ir/k/llvm-project/clang/test/AST/const-fpfeatures-diag.c Line 8: initializer element is not a compile-time constant
error: 'warning' diagnostics seen but not expected: 
  (frontend): overriding currently unsupported use of floating point exceptions on this target
2 errors generated.

--

********************
Testing:  0.. 10.. 20.. 30.. 40.. 50.. 60.. 70.. 80.. 90.. 
********************
Failed Tests (1):
  Clang :: AST/const-fpfeatures-diag.c

Would you mind taking a look and sending out a fix? Thanks.

[sepavloff]

Hi!

This issue must be fixed by: https://reviews.llvm.org/rGf91b9c0f9858
Do you have recent changes from the trunk?

Thanks,
--Serge

[leonardchan]

Thanks for looking into it. We have that commit but it still seems to be
failing for us with the same error.

[rjmccall]

Commented in that review, but that patch has the wrong fix: it's based on the targets LLVM is configured with rather than the current test target.

[sepavloff]

In D87822#2301194, @leonardchan wrote:

Thanks for looking into it. We have that commit but it still seems to be
failing for us with the same error.

In D88498 this test is removed because use of rounding mode changed. So you could just remove this test to fix the bot.

[phosek]

In D87822#2301293, @sepavloff wrote:

In D87822#2301194, @leonardchan wrote:

Thanks for looking into it. We have that commit but it still seems to be
failing for us with the same error.

In D88498 this test is removed because use of rounding mode changed. So you could just remove this test to fix the bot.

Can we revert this change and reland it later with the fix? It's been 3 days now and our bots are still red.

[sepavloff]

The change https://reviews.llvm.org/rG4e4f926e83cf removes the problematic
test.
Sorry for troubles.

Thanks,
--Serge

[joaomoreira]

I noticed that this commit breaks MUSL 1.2.0. Here is an isolated test-case that illustrates the issue:

https://godbolt.org/z/6bdnEh

clang/lib/AST/ExprConstant.cpp

static bool HandleFloatToIntCast(EvalInfo &Info, const Expr *E,
   // Determine whether we are converting to unsigned or signed.
   bool DestSigned = DestType->isSignedIntegerOrEnumerationType();
 
   Result = APSInt(DestWidth, !DestSigned);
   bool ignored;
   if (Value.convertToInteger(Result, llvm::APFloat::rmTowardZero, &ignored)
       & APFloat::opInvalidOp)
     return HandleOverflow(Info, E, Value, DestType);
   return true;
 }
 
 static bool HandleFloatToFloatCast(EvalInfo &Info, const Expr *E,
                                    QualType SrcType, QualType DestType,
                                    APFloat &Result) {
+  llvm::RoundingMode RM = llvm::RoundingMode::NearestTiesToEven;
+  if (auto Cast = dyn_cast<CastExpr>(E)) {
+    FPOptions FPFeatures = Cast->getFPFeaturesInEffect(Info.Ctx.getLangOpts());
+    RM = FPFeatures.getRoundingMode();
+  }
+  bool DynamicRM = RM == llvm::RoundingMode::Dynamic;
+  if (DynamicRM)
+    // If rounding mode is unknown at compile time, still try to execute the
+    // operation. If the result is exact, it does not depend on rounding mode.
+    RM = llvm::RoundingMode::NearestTiesToEven;
+
+  APFloat::opStatus St;
   APFloat Value = Result;
   bool ignored;
-  Result.convert(Info.Ctx.getFloatTypeSemantics(DestType),
-                 APFloat::rmNearestTiesToEven, &ignored);
+  St = Result.convert(Info.Ctx.getFloatTypeSemantics(DestType), RM, &ignored);
+
+  // FIXME: if:
+  // - evaluation triggered some FP exception, and
+  // - exception mode is not "ignore", and
+  // - the expression being evaluated is not a part of global variable
+  //   initializer,
+  // the evaluation probably need to be rejected.
+
   return true;
 }
 
 static APSInt HandleIntToIntCast(EvalInfo &Info, const Expr *E,
                                  QualType DestType, QualType SrcType,
                                  const APSInt &Value) {
   unsigned DestWidth = Info.Ctx.getIntWidth(DestType);
   // Figure out if this is a truncate, extend or noop cast.
   // If the input is signed, do a sign extend, noop, or truncate.
   APSInt Result = Value.extOrTrunc(DestWidth);
   Result.setIsUnsigned(DestType->isUnsignedIntegerOrEnumerationType());
   if (DestType->isBooleanType())
     Result = Value.getBoolValue();
   return Result;
 }
 
 static bool HandleIntToFloatCast(EvalInfo &Info, const Expr *E,
                                  QualType SrcType, const APSInt &Value,
                                  QualType DestType, APFloat &Result) {
   Result = APFloat(Info.Ctx.getFloatTypeSemantics(DestType), 1);
   Result.convertFromAPInt(Value, Value.isSigned(),
                           APFloat::rmNearestTiesToEven);
   return true;
 }
static bool handleIntIntBinOp(EvalInfo &Info, const Expr *E, const APSInt &LHS,
   case BO_GT: Result = LHS > RHS; return true;
   case BO_LE: Result = LHS <= RHS; return true;
   case BO_GE: Result = LHS >= RHS; return true;
   case BO_EQ: Result = LHS == RHS; return true;
   case BO_NE: Result = LHS != RHS; return true;
   case BO_Cmp:
     llvm_unreachable("BO_Cmp should be handled elsewhere");
   }
 }
 
 /// Perform the given binary floating-point operation, in-place, on LHS.
-static bool handleFloatFloatBinOp(EvalInfo &Info, const Expr *E,
+static bool handleFloatFloatBinOp(EvalInfo &Info, const BinaryOperator *E,
                                   APFloat &LHS, BinaryOperatorKind Opcode,
                                   const APFloat &RHS) {
+  FPOptions FPFeatures = E->getFPFeaturesInEffect(Info.Ctx.getLangOpts());
+  llvm::RoundingMode RM = FPFeatures.getRoundingMode();
+  bool DynamicRM = RM == llvm::RoundingMode::Dynamic;
+  if (DynamicRM)
+    // If rounding mode is unknown at compile time, still try to execute the
+    // operation. If the result is exact, it does not depend on rounding mode.
+    RM = llvm::RoundingMode::NearestTiesToEven;
+  APFloat::opStatus St;
   switch (Opcode) {
   default:
     Info.FFDiag(E);
     return false;
   case BO_Mul:
-    LHS.multiply(RHS, APFloat::rmNearestTiesToEven);
+    St = LHS.multiply(RHS, RM);
     break;
   case BO_Add:
-    LHS.add(RHS, APFloat::rmNearestTiesToEven);
+    St = LHS.add(RHS, RM);
     break;
   case BO_Sub:
-    LHS.subtract(RHS, APFloat::rmNearestTiesToEven);
+    St = LHS.subtract(RHS, RM);
     break;
   case BO_Div:
     // [expr.mul]p4:
     //   If the second operand of / or % is zero the behavior is undefined.
     if (RHS.isZero())
       Info.CCEDiag(E, diag::note_expr_divide_by_zero);
-    LHS.divide(RHS, APFloat::rmNearestTiesToEven);
+    St = LHS.divide(RHS, RM);
     break;
   }
 
+  if (DynamicRM && (St & APFloat::opInexact)) {
+    Info.FFDiag(E);
+    return false;
+  }
+
+  // FIXME: if:
+  // - evaluation triggered some FP exception, and
+  // - exception mode is not "ignore", and
+  // - the expression being evaluated is not a part of global variable
+  //   initializer,
+  // the evaluation probably need to be rejected.
+
   // [expr.pre]p4:
   //   If during the evaluation of an expression, the result is not
   //   mathematically defined [...], the behavior is undefined.
   // FIXME: C++ rules require us to not conform to IEEE 754 here.
   if (LHS.isNaN()) {
     Info.CCEDiag(E, diag::note_constexpr_float_arithmetic) << LHS.isNaN();
     return Info.noteUndefinedBehavior();
   }
if (LHSValue.getKind() == APValue::Int)
     return handleCompareOpForVectorHelper(LHSValue.getInt(), Opcode,
                                           RHSValue.getInt(), Result);
   assert(LHSValue.getKind() == APValue::Float && "Should be no other options");
   return handleCompareOpForVectorHelper(LHSValue.getFloat(), Opcode,
                                         RHSValue.getFloat(), Result);
 }
 
 // Perform binary operations for vector types, in place on the LHS.
-static bool handleVectorVectorBinOp(EvalInfo &Info, const Expr *E,
+static bool handleVectorVectorBinOp(EvalInfo &Info, const BinaryOperator *E,
                                     BinaryOperatorKind Opcode,
                                     APValue &LHSValue,
                                     const APValue &RHSValue) {
   assert(Opcode != BO_PtrMemD && Opcode != BO_PtrMemI &&
          "Operation not supported on vector types");
 
   const auto *VT = E->getType()->castAs<VectorType>();
   unsigned NumElements = VT->getNumElements();
static bool handleAssignment(EvalInfo &Info, const Expr *E, const LValue &LVal,
 
   CompleteObject Obj = findCompleteObject(Info, E, AK_Assign, LVal, LValType);
   return Obj && modifySubobject(Info, E, Obj, LVal.Designator, Val);
 }
 
 namespace {
 struct CompoundAssignSubobjectHandler {
   EvalInfo &Info;
-  const Expr *E;
+  const CompoundAssignOperator *E;
   QualType PromotedLHSType;
   BinaryOperatorKind Opcode;
   const APValue &RHS;
 
   static const AccessKinds AccessKind = AK_Assign;
 
   typedef bool result_type;
 
bool foundPointer(APValue &Subobj, QualType SubobjType) {
     return true;
   }
 };
 } // end anonymous namespace
 
 const AccessKinds CompoundAssignSubobjectHandler::AccessKind;
 
 /// Perform a compound assignment of LVal <op>= RVal.
-static bool handleCompoundAssignment(
-    EvalInfo &Info, const Expr *E,
-    const LValue &LVal, QualType LValType, QualType PromotedLValType,
-    BinaryOperatorKind Opcode, const APValue &RVal) {
+static bool handleCompoundAssignment(EvalInfo &Info,
+                                     const CompoundAssignOperator *E,
+                                     const LValue &LVal, QualType LValType,
+                                     QualType PromotedLValType,
+                                     BinaryOperatorKind Opcode,
+                                     const APValue &RVal) {
   if (LVal.Designator.Invalid)
     return false;
 
   if (!Info.getLangOpts().CPlusPlus14) {
     Info.FFDiag(E);
     return false;
   }
 

clang/lib/Sema/SemaAttr.cpp

void Sema::ActOnPragmaFPReassociate(SourceLocation Loc, bool IsEnabled) {
   NewFPFeatures.setAllowFPReassociateOverride(IsEnabled);
   FpPragmaStack.Act(Loc, PSK_Set, StringRef(), NewFPFeatures);
   CurFPFeatures = NewFPFeatures.applyOverrides(getLangOpts());
 }
 
 void Sema::setRoundingMode(SourceLocation Loc, llvm::RoundingMode FPR) {
   // C2x: 7.6.2p3  If the FE_DYNAMIC mode is specified and FENV_ACCESS is "off",
   // the translator may assume that the default rounding mode is in effect.
-  if (FPR == llvm::RoundingMode::Dynamic && !CurFPFeatures.getAllowFEnvAccess())
+  if (FPR == llvm::RoundingMode::Dynamic &&
+      !CurFPFeatures.getAllowFEnvAccess() &&
+      CurFPFeatures.getFPExceptionMode() == LangOptions::FPE_Ignore)
     FPR = llvm::RoundingMode::NearestTiesToEven;
 
   FPOptionsOverride NewFPFeatures = CurFPFeatureOverrides();
   NewFPFeatures.setRoundingModeOverride(FPR);
   FpPragmaStack.Act(Loc, PSK_Set, StringRef(), NewFPFeatures);
   CurFPFeatures = NewFPFeatures.applyOverrides(getLangOpts());
 }
 

clang/test/AST/const-fpfeatures-diag.c

+// RUN: %clang_cc1 -verify -ffp-exception-behavior=strict -Wno-unknown-pragmas %s
+
+#pragma STDC FENV_ROUND FE_DYNAMIC
+
+// nextUp(1.F) == 0x1.000002p0F
+
+float F1 = 0x1.000000p0F + 0x0.000002p0F;
+float F2 = 0x1.000000p0F + 0x0.000001p0F; // expected-error{{initializer element is not a compile-time constant}}

clang/test/AST/const-fpfeatures.c

+// RUN: %clang_cc1 -S -emit-llvm -Wno-unknown-pragmas %s -o - | FileCheck %s
+
+// nextUp(1.F) == 0x1.000002p0F
+
+#pragma STDC FENV_ROUND FE_UPWARD
+
+float F1u = 1.0F + 0x0.000002p0F;
+float F2u = 1.0F + 0x0.000001p0F;
+float F3u = 0x1.000001p0;
+// CHECK: @F1u = {{.*}} float 0x3FF0000020000000
+// CHECK: @F2u = {{.*}} float 0x3FF0000020000000
+// CHECK: @F3u = {{.*}} float 0x3FF0000020000000
+
+#pragma STDC FENV_ROUND FE_DOWNWARD
+
+float F1d = 1.0F + 0x0.000002p0F;
+float F2d = 1.0F + 0x0.000001p0F;
+float F3d = 0x1.000001p0;
+
+// CHECK: @F1d = {{.*}} float 0x3FF0000020000000
+// CHECK: @F2d = {{.*}} float 1.000000e+00
+// CHECK: @F3d = {{.*}} float 1.000000e+00