spatel created this revision.
spatel added reviewers: sepavloff, jyknight, efriedma, dim, kparzysz, rsmith.
Herald added a subscriber: mcrosier.
spatel requested review of this revision.

There's currently ongoing discussion on the dev lists about how to handle 
related cases with isnan() / isinf(), but I don't think the problem addressed 
by this patch is controversial:
If the compile specified loose FP math because special values like NaN / Inf / 
-0.0 are not expected, then we should warn if the code contains an obvious 
occurrence of any of those values.

My understanding of clang isn't good though. Is this the right place to detect 
the unexpected values? I commented in the test file that I don't see the 
expected warnings in all cases before CodeGen. But if we run through to IR 
creation, then I sometimes see duplicate warnings for the same line of code.

For example, I see duplicate 3 warnings on this program based on 
https://llvm.org/PR51775 :

  % cat 51775.c 
  #include <math.h>
  #include <stdlib.h>
  int main() {
      const double d = strtod("1E+1000000", NULL);
      return d == HUGE_VAL;
  }



  % clang -O1 -ffast-math 51775.c -S -o -
  51775.c:5:17: warning: floating-point infinity may be optimized out of 
computation or comparison [-Wliteral-range]
      return d == HUGE_VAL;
                  ^
  
/Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX.sdk/usr/include/math.h:57:28:
 note: expanded from macro 'HUGE_VAL'
  #   define    HUGE_VAL     __builtin_huge_val()
                             ^
  51775.c:5:17: warning: floating-point infinity may be optimized out of 
computation or comparison [-Wliteral-range]
  
/Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX.sdk/usr/include/math.h:57:28:
 note: expanded from macro 'HUGE_VAL'
  #   define    HUGE_VAL     __builtin_huge_val()
                             ^
  51775.c:5:17: warning: floating-point infinity may be optimized out of 
computation or comparison [-Wliteral-range]
  
/Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX.sdk/usr/include/math.h:57:28:
 note: expanded from macro 'HUGE_VAL'
  #   define    HUGE_VAL     __builtin_huge_val()
  ...


https://reviews.llvm.org/D109925

Files:
  clang/include/clang/Basic/DiagnosticASTKinds.td
  clang/lib/AST/ExprConstant.cpp
  clang/test/AST/warn-fp-values.c

Index: clang/test/AST/warn-fp-values.c
===================================================================
--- /dev/null
+++ clang/test/AST/warn-fp-values.c
@@ -0,0 +1,50 @@
+// RUN: %clang_cc1 %s -triple x86_64-unknown-unknown -verify -fsyntax-only -ffast-math -Wliteral-range
+
+#define INFINITY_MATH_MACRO  1.0 / 0.0
+
+#define NAN_MATH_MACRO  0.0f / 0.0f
+
+// Infinity
+
+double inf_by_macro() {
+  double d = INFINITY_MATH_MACRO; // expected-warning {{floating-point infinity may be optimized}}
+  return d;
+}
+
+float inf_by_overflow() {
+  const float huge_literal = 10e100f; // expected-warning {{magnitude of floating-point constant too large}} {{floating-point infinity may be optimized}}
+  return huge_literal;
+}
+
+double inf_by_builtin() {
+  double b = __builtin_inf(); // shows warning if compile continues with -emit-llvm
+  return b;
+}
+
+float inf_by_builtin_cast() {
+  float b = __builtin_inf(); // expected-warning {{floating-point infinity may be optimized}}
+  return b;
+}
+
+// NaN
+
+float nan_by_macro() {
+  float f = NAN_MATH_MACRO; // expected-warning {{floating-point NaN may be optimized}}
+  return f;
+}
+
+double nan_by_builtin() {
+  double b = __builtin_nan(""); // shows warning if compile continues with -emit-llvm
+  return b;
+}
+
+float nan_by_builtin_cast() {
+  float b = __builtin_nan(""); // expected-warning {{floating-point NaN may be optimized}}
+  return b;
+}
+
+// -0.0
+
+double neg_zero_literal() {
+  return -0.0; // shows warning if compile continues with -emit-llvm
+}
Index: clang/lib/AST/ExprConstant.cpp
===================================================================
--- clang/lib/AST/ExprConstant.cpp
+++ clang/lib/AST/ExprConstant.cpp
@@ -13563,10 +13563,22 @@
 };
 } // end anonymous namespace
 
-static bool EvaluateFloat(const Expr* E, APFloat& Result, EvalInfo &Info) {
+static bool EvaluateFloat(const Expr *E, APFloat &Result, EvalInfo &Info) {
   assert(!E->isValueDependent());
   assert(E->isPRValue() && E->getType()->isRealFloatingType());
-  return FloatExprEvaluator(Info, Result).Visit(E);
+  bool Status = FloatExprEvaluator(Info, Result).Visit(E);
+
+  // Warn if we encounter a floating-point special value that is potentially
+  // constant-folded by the optimizer.
+  const FPOptions &FPO = E->getFPFeaturesInEffect(Info.Ctx.getLangOpts());
+  if (Result.isInfinity() && FPO.getNoHonorInfs())
+    Info.Ctx.getDiagnostics().Report(E->getExprLoc(), diag::warn_fp_infinity);
+  else if (Result.isNaN() && FPO.getNoHonorNaNs())
+    Info.Ctx.getDiagnostics().Report(E->getExprLoc(), diag::warn_fp_nan);
+  else if (Result.isNegZero() && FPO.getNoSignedZero())
+    Info.Ctx.getDiagnostics().Report(E->getExprLoc(), diag::warn_fp_negzero);
+
+  return Status;
 }
 
 static bool TryEvaluateBuiltinNaN(const ASTContext &Context,
Index: clang/include/clang/Basic/DiagnosticASTKinds.td
===================================================================
--- clang/include/clang/Basic/DiagnosticASTKinds.td
+++ clang/include/clang/Basic/DiagnosticASTKinds.td
@@ -371,6 +371,15 @@
 def warn_fixedpoint_constant_overflow : Warning<
   "overflow in expression; result is %0 with type %1">,
   InGroup<DiagGroup<"fixed-point-overflow">>;
+def warn_fp_infinity : Warning<
+  "floating-point infinity may be optimized out of computation or comparison">,
+  InGroup<LiteralRange>;
+def warn_fp_nan: Warning<
+  "floating-point NaN may be optimized out of computation or comparison">,
+  InGroup<LiteralRange>;
+def warn_fp_negzero: Warning<
+  "floating-point -0.0 may be optimized out of computation or comparison">,
+  InGroup<LiteralRange>;
 
 // This is a temporary diagnostic, and shall be removed once our
 // implementation is complete, and like the preceding constexpr notes belongs
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
  • [PATCH] D109925: [AST] add wa... Sanjay Patel via Phabricator via cfe-commits

Reply via email to