[clang] [CLANG] Add warning when comparing to INF or NAN in fast math mode. (PR #76873)

via cfe-commits cfe-commits at lists.llvm.org
Mon Jan 8 05:09:47 PST 2024


llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-clang

Author: Zahira Ammarguellat (zahiraam)

<details>
<summary>Changes</summary>

Check for comparisons to INF and NaN when in ffast-math mode and generate a warning.

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


5 Files Affected:

- (modified) clang/include/clang/Basic/DiagnosticSemaKinds.td (+3) 
- (modified) clang/include/clang/Sema/Sema.h (+7-2) 
- (modified) clang/lib/Sema/SemaChecking.cpp (+74-6) 
- (modified) clang/lib/Sema/SemaExpr.cpp (+5-2) 
- (added) clang/test/Sema/warn-fp-fast-compare.cpp (+245) 


``````````diff
diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index e54f969c19039d..d19be567a1bf66 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -6771,6 +6771,9 @@ def warn_pointer_sub_null_ptr : Warning<
 def warn_floatingpoint_eq : Warning<
   "comparing floating point with == or != is unsafe">,
   InGroup<DiagGroup<"float-equal">>, DefaultIgnore;
+def warn_fast_floatingpoint_eq : Warning<
+  "explicit comparison with %0 when the program is assumed to not use or produce %0">,
+  InGroup<TautologicalConstantCompare>;
 
 def err_setting_eval_method_used_in_unsafe_context : Error <
   "%select{'#pragma clang fp eval_method'|option 'ffp-eval-method'}0 cannot be used with "
diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
index 5e3b57ea33220b..b19ce6312e83bc 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -13893,8 +13893,9 @@ class Sema final {
 
   bool SemaBuiltinVAStart(unsigned BuiltinID, CallExpr *TheCall);
   bool SemaBuiltinVAStartARMMicrosoft(CallExpr *Call);
-  bool SemaBuiltinUnorderedCompare(CallExpr *TheCall);
-  bool SemaBuiltinFPClassification(CallExpr *TheCall, unsigned NumArgs);
+  bool SemaBuiltinUnorderedCompare(CallExpr *TheCall, unsigned BuiltinID);
+  bool SemaBuiltinFPClassification(CallExpr *TheCall, unsigned NumArgs,
+                                   unsigned BuiltinID);
   bool SemaBuiltinComplex(CallExpr *TheCall);
   bool SemaBuiltinVSX(CallExpr *TheCall);
   bool SemaBuiltinOSLogFormat(CallExpr *TheCall);
@@ -13998,6 +13999,8 @@ class Sema final {
                             SourceRange range,
                             llvm::SmallBitVector &CheckedVarArgs);
 
+  void CheckInfNaNFunction(const CallExpr *Call, const FunctionDecl *FDecl);
+
   void CheckAbsoluteValueFunction(const CallExpr *Call,
                                   const FunctionDecl *FDecl);
 
@@ -14024,6 +14027,8 @@ class Sema final {
 public:
   void CheckFloatComparison(SourceLocation Loc, Expr *LHS, Expr *RHS,
                             BinaryOperatorKind Opcode);
+  void CheckInfNaNFloatComparison(SourceLocation Loc, Expr *LHS, Expr *RHS,
+                                  BinaryOperatorKind Opcode);
 
 private:
   void CheckImplicitConversions(Expr *E, SourceLocation CC = SourceLocation());
diff --git a/clang/lib/Sema/SemaChecking.cpp b/clang/lib/Sema/SemaChecking.cpp
index 3168d38dd66c36..2c4d628ab160d2 100644
--- a/clang/lib/Sema/SemaChecking.cpp
+++ b/clang/lib/Sema/SemaChecking.cpp
@@ -2169,6 +2169,7 @@ Sema::CheckBuiltinFunctionCall(FunctionDecl *FDecl, unsigned BuiltinID,
     ICEArguments &= ~(1 << ArgNo);
   }
 
+  FPOptions FPO;
   switch (BuiltinID) {
   case Builtin::BI__builtin___CFStringMakeConstantString:
     // CFStringMakeConstantString is currently not implemented for GOFF (i.e.,
@@ -2245,15 +2246,15 @@ Sema::CheckBuiltinFunctionCall(FunctionDecl *FDecl, unsigned BuiltinID,
   case Builtin::BI__builtin_islessequal:
   case Builtin::BI__builtin_islessgreater:
   case Builtin::BI__builtin_isunordered:
-    if (SemaBuiltinUnorderedCompare(TheCall))
+    if (SemaBuiltinUnorderedCompare(TheCall, BuiltinID))
       return ExprError();
     break;
   case Builtin::BI__builtin_fpclassify:
-    if (SemaBuiltinFPClassification(TheCall, 6))
+    if (SemaBuiltinFPClassification(TheCall, 6, BuiltinID))
       return ExprError();
     break;
   case Builtin::BI__builtin_isfpclass:
-    if (SemaBuiltinFPClassification(TheCall, 2))
+    if (SemaBuiltinFPClassification(TheCall, 2, BuiltinID))
       return ExprError();
     break;
   case Builtin::BI__builtin_isfinite:
@@ -2267,7 +2268,7 @@ Sema::CheckBuiltinFunctionCall(FunctionDecl *FDecl, unsigned BuiltinID,
   case Builtin::BI__builtin_signbit:
   case Builtin::BI__builtin_signbitf:
   case Builtin::BI__builtin_signbitl:
-    if (SemaBuiltinFPClassification(TheCall, 1))
+    if (SemaBuiltinFPClassification(TheCall, 1, BuiltinID))
       return ExprError();
     break;
   case Builtin::BI__builtin_shufflevector:
@@ -7621,6 +7622,7 @@ bool Sema::CheckFunctionCall(FunctionDecl *FDecl, CallExpr *TheCall,
 
   CheckAbsoluteValueFunction(TheCall, FDecl);
   CheckMaxUnsignedZero(TheCall, FDecl);
+  CheckInfNaNFunction(TheCall, FDecl);
 
   if (getLangOpts().ObjC)
     DiagnoseCStringFormatDirectiveInCFAPI(*this, FDecl, Args, NumArgs);
@@ -9090,10 +9092,16 @@ bool Sema::SemaBuiltinVAStartARMMicrosoft(CallExpr *Call) {
 
 /// SemaBuiltinUnorderedCompare - Handle functions like __builtin_isgreater and
 /// friends.  This is declared to take (...), so we have to check everything.
-bool Sema::SemaBuiltinUnorderedCompare(CallExpr *TheCall) {
+bool Sema::SemaBuiltinUnorderedCompare(CallExpr *TheCall, unsigned BuiltinID) {
   if (checkArgCount(*this, TheCall, 2))
     return true;
 
+  if (BuiltinID == Builtin::BI__builtin_isunordered) {
+    if (TheCall->getFPFeaturesInEffect(getLangOpts()).getNoHonorNaNs())
+      Diag(TheCall->getBeginLoc(), diag::warn_fast_floatingpoint_eq)
+          << "NaN" << TheCall->getSourceRange();
+  }
+
   ExprResult OrigArg0 = TheCall->getArg(0);
   ExprResult OrigArg1 = TheCall->getArg(1);
 
@@ -9128,10 +9136,22 @@ bool Sema::SemaBuiltinUnorderedCompare(CallExpr *TheCall) {
 /// SemaBuiltinSemaBuiltinFPClassification - Handle functions like
 /// __builtin_isnan and friends.  This is declared to take (...), so we have
 /// to check everything.
-bool Sema::SemaBuiltinFPClassification(CallExpr *TheCall, unsigned NumArgs) {
+bool Sema::SemaBuiltinFPClassification(CallExpr *TheCall, unsigned NumArgs,
+                                       unsigned BuiltinID) {
   if (checkArgCount(*this, TheCall, NumArgs))
     return true;
 
+  FPOptions FPO = TheCall->getFPFeaturesInEffect(getLangOpts());
+  if (FPO.getNoHonorInfs() && (BuiltinID == Builtin::BI__builtin_isfinite ||
+                               BuiltinID == Builtin::BI__builtin_isinf ||
+                               BuiltinID == Builtin::BI__builtin_isinf_sign))
+    Diag(TheCall->getBeginLoc(), diag::warn_fast_floatingpoint_eq)
+        << "infinity" << TheCall->getSourceRange();
+  if (FPO.getNoHonorNaNs() && (BuiltinID == Builtin::BI__builtin_isnan ||
+                               BuiltinID == Builtin::BI__builtin_isunordered))
+    Diag(TheCall->getBeginLoc(), diag::warn_fast_floatingpoint_eq)
+        << "NaN" << TheCall->getSourceRange();
+
   bool IsFPClass = NumArgs == 2;
 
   // Find out position of floating-point argument.
@@ -12878,6 +12898,22 @@ static bool IsStdFunction(const FunctionDecl *FDecl,
   return true;
 }
 
+void Sema::CheckInfNaNFunction(const CallExpr *Call,
+                               const FunctionDecl *FDecl) {
+  FPOptions FPO = Call->getFPFeaturesInEffect(getLangOpts());
+  if ((IsStdFunction(FDecl, "isnan") || IsStdFunction(FDecl, "isunordered") ||
+       (Call->getBuiltinCallee() == Builtin::BI__builtin_nanf)) &&
+      FPO.getNoHonorNaNs())
+    Diag(Call->getBeginLoc(), diag::warn_fast_floatingpoint_eq)
+        << "NaN" << Call->getSourceRange();
+  else if ((IsStdFunction(FDecl, "isinf") ||
+            (IsStdFunction(FDecl, "isfinite") ||
+             (Call->getBuiltinCallee() == Builtin::BI__builtin_inff))) &&
+           FPO.getNoHonorInfs())
+    Diag(Call->getBeginLoc(), diag::warn_fast_floatingpoint_eq)
+        << "infinity" << Call->getSourceRange();
+}
+
 // Warn when using the wrong abs() function.
 void Sema::CheckAbsoluteValueFunction(const CallExpr *Call,
                                       const FunctionDecl *FDecl) {
@@ -13846,6 +13882,38 @@ Sema::CheckReturnValExpr(Expr *RetValExp, QualType lhsType,
     CheckPPCMMAType(RetValExp->getType(), ReturnLoc);
 }
 
+/// Diagnose comparison to NAN or INFINITY in fast math modes.
+/// The comparison to NaN or INFINITY is always false in
+/// fast modes: float evaluation will not result in inf or nan.
+void Sema::CheckInfNaNFloatComparison(SourceLocation Loc, Expr *LHS, Expr *RHS,
+                                      BinaryOperatorKind Opcode) {
+  Expr *LeftExprSansParen = LHS->IgnoreParenImpCasts();
+  Expr *RightExprSansParen = RHS->IgnoreParenImpCasts();
+
+  FPOptions FPO = LHS->getFPFeaturesInEffect(getLangOpts());
+  bool NoHonorNaNs = FPO.getNoHonorNaNs();
+  bool NoHonorInfs = FPO.getNoHonorInfs();
+  llvm::APFloat Value(0.0);
+
+  auto IsConstant = [&Value](Expr *E, Expr *ESansParen, ASTContext &Context) {
+    return !E->isValueDependent() &&
+           ESansParen->EvaluateAsFloat(Value, Context,
+                                       Expr::SE_AllowSideEffects);
+  };
+
+  if (IsConstant(LHS, LeftExprSansParen, Context) &&
+      ((NoHonorNaNs && Value.isNaN()) || (NoHonorInfs && Value.isInfinity())))
+    Diag(Loc, diag::warn_fast_floatingpoint_eq)
+        << (Value.isNaN() ? "NaN" : "infinity") << LHS->getSourceRange()
+        << RHS->getSourceRange();
+
+  if (IsConstant(RHS, RightExprSansParen, Context) &&
+      ((NoHonorNaNs && Value.isNaN()) || (NoHonorInfs && Value.isInfinity())))
+    Diag(Loc, diag::warn_fast_floatingpoint_eq)
+        << (Value.isNaN() ? "NaN" : "infinity") << LHS->getSourceRange()
+        << RHS->getSourceRange();
+}
+
 /// Check for comparisons of floating-point values using == and !=. Issue a
 /// warning if the comparison is not likely to do what the programmer intended.
 void Sema::CheckFloatComparison(SourceLocation Loc, Expr *LHS, Expr *RHS,
diff --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp
index 960f513d1111b2..005ddfa882195d 100644
--- a/clang/lib/Sema/SemaExpr.cpp
+++ b/clang/lib/Sema/SemaExpr.cpp
@@ -13044,9 +13044,12 @@ static QualType checkArithmeticOrEnumeralCompare(Sema &S, ExprResult &LHS,
   if (Type->isAnyComplexType() && BinaryOperator::isRelationalOp(Opc))
     return S.InvalidOperands(Loc, LHS, RHS);
 
-  // Check for comparisons of floating point operands using != and ==.
-  if (Type->hasFloatingRepresentation())
+  if (Type->hasFloatingRepresentation()) {
+    // Check for comparisons to NAN or INFINITY in fast math mode.
+    S.CheckInfNaNFloatComparison(Loc, LHS.get(), RHS.get(), Opc);
+    // Check for comparisons of floating point operands using != and ==.
     S.CheckFloatComparison(Loc, LHS.get(), RHS.get(), Opc);
+  }
 
   // The result of comparisons is 'bool' in C++, 'int' in C.
   return S.Context.getLogicalOperationType();
diff --git a/clang/test/Sema/warn-fp-fast-compare.cpp b/clang/test/Sema/warn-fp-fast-compare.cpp
new file mode 100644
index 00000000000000..df8c8145f538ad
--- /dev/null
+++ b/clang/test/Sema/warn-fp-fast-compare.cpp
@@ -0,0 +1,245 @@
+// RUN: %clang_cc1 -x c++ -verify -triple powerpc64le-unknown-unknown %s \
+// RUN: -menable-no-infs -menable-no-nans  -DFAST=1
+
+// RUN: %clang_cc1 -x c++ -verify -triple powerpc64le-unknown-unknown %s \
+// RUN: -DNOFAST=1
+
+// RUN: %clang_cc1 -x c++ -verify -triple powerpc64le-unknown-unknown %s \
+// RUN: -menable-no-infs -DNO_INFS=1
+
+// RUN: %clang_cc1 -x c++ -verify -triple powerpc64le-unknown-unknown %s \
+// RUN: -menable-no-nans -DNO_NANS=1
+
+int isunorderedf (float x, float y);
+#if NOFAST
+// expected-no-diagnostics
+#endif
+extern "C++" {
+namespace std __attribute__((__visibility__("default"))) {
+  bool
+  isinf(float __x);
+  bool
+  isinf(double __x);
+  bool
+  isinf(long double __x);
+  bool
+  isnan(float __x);
+  bool
+  isnan(double __x);
+  bool
+  isnan(long double __x);
+bool
+  isfinite(float __x);
+  bool
+  isfinite(double __x);
+  bool
+  isfinte(long double __x);
+ bool
+  isunordered(float __x, float __y);
+  bool
+  isunordered(double __x, double __y);
+  bool
+  isunordered(long double __x, long double __y);
+} // namespace )
+}
+#define NAN (__builtin_nanf(""))
+#define INFINITY (__builtin_inff())
+
+int compareit(float a, float b) {
+  volatile int i, j, k, l, m, n, o, p;
+#if FAST
+// expected-warning at +11 {{explicit comparison with infinity when the program is assumed to not use or produce infinity}}
+#endif
+#if FAST
+// expected-warning at +8 {{explicit comparison with infinity when the program is assumed to not use or produce infinity}}
+#endif
+#if NO_INFS
+// expected-warning at +5 {{explicit comparison with infinity when the program is assumed to not use or produce infinity}}
+#endif
+#if NO_INFS
+// expected-warning at +2 {{explicit comparison with infinity when the program is assumed to not use or produce infinity}}
+#endif
+  i = a == INFINITY;
+#if FAST
+// expected-warning at +11 {{explicit comparison with infinity when the program is assumed to not use or produce infinity}}
+#endif
+#if FAST
+// expected-warning at +8 {{explicit comparison with infinity when the program is assumed to not use or produce infinity}}
+#endif
+#if NO_INFS
+// expected-warning at +5 {{explicit comparison with infinity when the program is assumed to not use or produce infinity}}
+#endif
+#if NO_INFS
+// expected-warning at +2 {{explicit comparison with infinity when the program is assumed to not use or produce infinity}}
+#endif
+  j = INFINITY == a;
+#if FAST
+// expected-warning at +11 {{explicit comparison with NaN when the program is assumed to not use or produce NaN}}
+#endif
+#if FAST
+// expected-warning at +8 {{explicit comparison with NaN when the program is assumed to not use or produce NaN}}
+#endif
+#if NO_NANS
+// expected-warning at +5 {{explicit comparison with NaN when the program is assumed to not use or produce NaN}}
+#endif
+#if NO_NANS
+// expected-warning at +2 {{explicit comparison with NaN when the program is assumed to not use or produce NaN}}
+#endif
+  i = a == NAN;
+#if FAST
+// expected-warning at +11 {{explicit comparison with NaN when the program is assumed to not use or produce NaN}}
+#endif
+#if FAST
+// expected-warning at +8 {{explicit comparison with NaN when the program is assumed to not use or produce NaN}}
+#endif
+#if NO_NANS
+// expected-warning at +5 {{explicit comparison with NaN when the program is assumed to not use or produce NaN}}
+#endif
+#if NO_NANS
+// expected-warning at +2 {{explicit comparison with NaN when the program is assumed to not use or produce NaN}}
+#endif
+  j = NAN == a;
+#if FAST
+// expected-warning at +11 {{explicit comparison with infinity when the program is assumed to not use or produce infinity}}
+#endif
+#if FAST
+// expected-warning at +8 {{explicit comparison with infinity when the program is assumed to not use or produce infinity}}
+#endif
+#if NO_INFS
+// expected-warning at +5 {{explicit comparison with infinity when the program is assumed to not use or produce infinity}}
+#endif
+#if NO_INFS
+// expected-warning at +2 {{explicit comparison with infinity when the program is assumed to not use or produce infinity}}
+#endif
+  j = INFINITY <= a;
+#if FAST
+// expected-warning at +11 {{explicit comparison with infinity when the program is assumed to not use or produce infinity}}
+#endif
+#if FAST
+// expected-warning at +8 {{explicit comparison with infinity when the program is assumed to not use or produce infinity}}
+#endif
+#if NO_INFS
+  // expected-warning at +5 {{explicit comparison with infinity when the program is assumed to not use or produce infinity}}
+#endif
+#if NO_INFS
+// expected-warning at +2 {{explicit comparison with infinity when the program is assumed to not use or produce infinity}}
+#endif
+  j = INFINITY < a;
+#if FAST
+// expected-warning at +11 {{explicit comparison with NaN when the program is assumed to not use or produce NaN}}
+#endif
+#if FAST
+// expected-warning at +8 {{explicit comparison with NaN when the program is assumed to not use or produce NaN}}
+#endif
+#if NO_NANS
+// expected-warning at +5 {{explicit comparison with NaN when the program is assumed to not use or produce NaN}}
+#endif
+#if NO_NANS
+  // expected-warning at +2 {{explicit comparison with NaN when the program is assumed to not use or produce NaN}}
+#endif
+  j = a > NAN;
+#if FAST
+// expected-warning at +11 {{explicit comparison with NaN when the program is assumed to not use or produce NaN}}
+#endif
+#if FAST
+// expected-warning at +8 {{explicit comparison with NaN when the program is assumed to not use or produce NaN}}
+#endif
+#if NO_NANS
+// expected-warning at +5 {{explicit comparison with NaN when the program is assumed to not use or produce NaN}}
+#endif
+#if NO_NANS
+// expected-warning at +2 {{explicit comparison with NaN when the program is assumed to not use or produce NaN}}
+#endif
+  j = a >= NAN;
+#if FAST
+// expected-warning at +5 {{explicit comparison with infinity when the program is assumed to not use or produce infinity}}
+#endif
+#if NO_INFS
+// expected-warning at +2 {{explicit comparison with infinity when the program is assumed to not use or produce infinity}}
+#endif
+k = std::isinf(a);
+#if FAST
+// expected-warning at +5 {{explicit comparison with NaN when the program is assumed to not use or produce NaN}}
+#endif
+#if NO_NANS
+// expected-warning at +2 {{explicit comparison with NaN when the program is assumed to not use or produce NaN}}
+#endif
+  l = std::isnan(a);
+#if FAST
+// expected-warning at +5 {{explicit comparison with infinity when the program is assumed to not use or produce infinity}}
+#endif
+#if NO_INFS
+// expected-warning at +2 {{explicit comparison with infinity when the program is assumed to not use or produce infinity}}
+#endif
+  o = std::isfinite(a);
+#if FAST
+// expected-warning at +5 {{explicit comparison with infinity when the program is assumed to not use or produce infinity}}
+#endif
+#if NO_INFS
+// expected-warning at +2 {{explicit comparison with infinity when the program is assumed to not use or produce infinity}}
+#endif
+  m = __builtin_isinf(a);
+#if FAST
+// expected-warning at +5 {{explicit comparison with NaN when the program is assumed to not use or produce NaN}}
+#endif
+#if NO_NANS
+// expected-warning at +2 {{explicit comparison with NaN when the program is assumed to not use or produce NaN}}
+#endif
+  n = __builtin_isnan(a);
+#if FAST
+// expected-warning at +5 {{explicit comparison with infinity when the program is assumed to not use or produce infinity}}
+#endif
+#if NO_INFS
+// expected-warning at +2 {{explicit comparison with infinity when the program is assumed to not use or produce infinity}}
+#endif
+  p = __builtin_isfinite(a);
+
+  // These should NOT warn, since they are not comparing with NaN or infinity.
+  j = a > 1.1;
+  j = b < 1.1;
+  j = a >= 1.1;
+  j = b <= 1.1;
+  j = isunorderedf(a, b);
+
+#if FAST
+// expected-warning at +5 {{explicit comparison with NaN when the program is assumed to not use or produce NaN}}
+#endif
+#if NO_NANS
+// expected-warning at +2 {{explicit comparison with NaN when the program is assumed to not use or produce NaN}}
+#endif
+  j = isunorderedf(a, NAN);
+#if FAST
+// expected-warning at +5 {{explicit comparison with infinity when the program is assumed to not use or produce infinity}}
+#endif
+#if NO_INFS
+// expected-warning at +2 {{explicit comparison with infinity when the program is assumed to not use or produce infinity}}
+#endif
+  j = isunorderedf(a, INFINITY);
+#if FAST
+// expected-warning at +11 {{explicit comparison with NaN when the program is assumed to not use or produce NaN}}
+#endif
+#if FAST
+// expected-warning at +8 {{explicit comparison with NaN when the program is assumed to not use or produce NaN}}
+#endif
+#if NO_NANS
+// expected-warning at +5 {{explicit comparison with NaN when the program is assumed to not use or produce NaN}}
+#endif
+#if NO_NANS
+// expected-warning at +2 {{explicit comparison with NaN when the program is assumed to not use or produce NaN}}
+#endif
+  i = std::isunordered(a, NAN);
+#if FAST
+// expected-warning at +11 {{explicit comparison with infinity when the program is assumed to not use or produce infinity}}
+#endif
+#if FAST
+// expected-warning at +8 {{explicit comparison with NaN when the program is assumed to not use or produce NaN}}
+#endif
+#if NO_NANS
+// expected-warning at +5 {{explicit comparison with NaN when the program is assumed to not use or produce NaN}}
+#endif
+#if NO_INFS
+// expected-warning at +2 {{explicit comparison with infinity when the program is assumed to not use or produce infinity}}
+#endif
+  i = std::isunordered(a, INFINITY);
+  return 0;
+}  

``````````

</details>


https://github.com/llvm/llvm-project/pull/76873


More information about the cfe-commits mailing list