zahiraam created this revision.
zahiraam added reviewers: andrew.w.kaylor, aaron.ballman, thakis, rjmccall, 
fhahn.
Herald added a project: All.
zahiraam requested review of this revision.
Herald added a project: clang.

Currently the control of the eval-method is mixed with fast-math.  
FLT_EVAL_METHOD tells the user the precision at which, temporary results
are evaluated but when fast-math is enabled, the numeric values are not     
guaranteed to match the source semantics, so the eval-method is
meaningless.
For example, the expression `x + y + z` has as source semantics `(x + y) + z`. 
FLT_EVAL_METHOD is telling the user at which precision `(x + y)`
is evaluated. With fast-math enable the compiler may choose to evaluate the 
expression as `(y + z) + x`.

The correct behavior is to set the FLT_EVAL_METHOD to `-1`, to tell the user 
that the precision of the intermediate values is unknow. 
This patch is doing that.

This patch is also fixing a crash when the `pragma clang fp  eval-method` is 
given the wrong value (should be source, double, extended).


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D121122

Files:
  clang/include/clang/Basic/DiagnosticParseKinds.td
  clang/lib/Lex/PPMacroExpansion.cpp
  clang/lib/Sema/Sema.cpp
  clang/lib/Sema/SemaAttr.cpp
  clang/test/CodeGen/eval-method-fast-math.c
  clang/test/Sema/fp-eval-pragma.cpp

Index: clang/test/Sema/fp-eval-pragma.cpp
===================================================================
--- clang/test/Sema/fp-eval-pragma.cpp
+++ clang/test/Sema/fp-eval-pragma.cpp
@@ -27,6 +27,16 @@
   return 0;
 }
 
+void apply_pragma_with_wrong_value() {
+ // expected-error@+1{{unexpected argument 'value' to '#pragma clang fp eval_method'; expected 'source' or 'double' or 'extended'}}
+#pragma clang fp eval_method(value)
+}
+
+int foo3() {
+  apply_pragma_with_wrong_value();
+  return 0;
+}
+
 void foo() {
   auto a = __FLT_EVAL_METHOD__;
   {
Index: clang/test/CodeGen/eval-method-fast-math.c
===================================================================
--- /dev/null
+++ clang/test/CodeGen/eval-method-fast-math.c
@@ -0,0 +1,55 @@
+// RUN: %clang_cc1 -fexperimental-strict-floating-point  \
+// RUN: -triple x86_64-linux-gnu -emit-llvm -o - %s  \
+// RUN: | FileCheck %s -check-prefixes=CHECK
+
+// RUN: %clang_cc1 -triple i386--linux -emit-llvm -o - %s \
+// RUN: | FileCheck %s -check-prefixes=CHECK-EXT
+
+// RUN: %clang_cc1 -fexperimental-strict-floating-point  \
+// RUN: -mreassociate -freciprocal-math -ffp-contract=fast \
+// RUN: -ffast-math -triple x86_64-linux-gnu \
+// RUN: -emit-llvm -o - %s \
+// RUN: | FileCheck %s -check-prefixes=CHECK-FAST
+
+// RUN: %clang_cc1 -triple i386--linux -mreassociate -freciprocal-math \
+// RUN: -ffp-contract=fast -ffast-math -emit-llvm -o - %s \
+// RUN: | FileCheck %s -check-prefixes=CHECK-FAST
+
+float res;
+int add(float a, float b, float c) {
+  // CHECK: fadd float
+  // CHECK: load float, float*
+  // CHECK: fadd float
+  // CHECK: store float
+  // CHECK-FAST: fadd fast float
+  // CHECK-FAST: load float, float*
+  // CHECK-FAST: fadd fast float
+  // CHECK: ret i32 0
+  // CHECK-EXT: ret i32 2
+  // CHECK-FAST: ret i32 -1
+  res = a + b + c;
+  return __FLT_EVAL_METHOD__;
+}
+
+int add_precise(float a, float b, float c) {
+#pragma float_control(precise, on)
+  // CHECK: fadd float
+  // CHECK: load float, float*
+  // CHECK: fadd float
+  // CHECK: store float
+  // CHECK: ret i32 0
+  // CHECK-FAST: ret i32 -1
+  res = a + b + c;
+  return __FLT_EVAL_METHOD__;
+}
+
+int add_not_precise(float a, float b, float c) {
+  // Fast-math is enabled with this pragma.
+#pragma float_control(precise, off)
+  // CHECK-FAST: fadd fast float
+  // CHECK-FAST: load float, float*
+  // CHECK-FAST: fadd fast float
+  // CHECK-FAST: ret i32 -1
+  res = a + b + c;
+  return __FLT_EVAL_METHOD__;
+}
Index: clang/lib/Sema/SemaAttr.cpp
===================================================================
--- clang/lib/Sema/SemaAttr.cpp
+++ clang/lib/Sema/SemaAttr.cpp
@@ -517,6 +517,10 @@
     else
       NewFPFeatures.setFPPreciseEnabled(false);
     FpPragmaStack.Act(Loc, Action, StringRef(), NewFPFeatures);
+    // Fast-math is enabled.
+    PP.setCurrentFPEvalMethod(
+        Loc, LangOptions::FPEvalMethodKind::FEM_Indeterminable);
+    PP.getCurrentFPEvalMethod();
     break;
   case PFC_Except:
     if (!isPreciseFPEnabled())
Index: clang/lib/Sema/Sema.cpp
===================================================================
--- clang/lib/Sema/Sema.cpp
+++ clang/lib/Sema/Sema.cpp
@@ -254,6 +254,9 @@
     PP.setCurrentFPEvalMethod(SourceLocation(),
                               getLangOpts().getFPEvalMethod());
   CurFPFeatures.setFPEvalMethod(PP.getCurrentFPEvalMethod());
+  // Fast-math is enabled.
+  if (getLangOpts().AllowFPReassoc || getLangOpts().AllowRecip)
+    PP.setCurrentFPEvalMethod(SourceLocation(), LangOptions::FEM_Indeterminable);
 }
 
 // Anchor Sema's type info to this TU.
Index: clang/lib/Lex/PPMacroExpansion.cpp
===================================================================
--- clang/lib/Lex/PPMacroExpansion.cpp
+++ clang/lib/Lex/PPMacroExpansion.cpp
@@ -1577,14 +1577,35 @@
     Tok.setKind(tok::string_literal);
   } else if (II == Ident__FLT_EVAL_METHOD__) {
     // __FLT_EVAL_METHOD__ is set to the default value.
-    OS << getTUFPEvalMethod();
-    // __FLT_EVAL_METHOD__ expands to a simple numeric value.
-    Tok.setKind(tok::numeric_constant);
-    if (getLastFPEvalPragmaLocation().isValid()) {
-      // The program is ill-formed. The value of __FLT_EVAL_METHOD__ is altered
-      // by the pragma.
-      Diag(Tok, diag::err_illegal_use_of_flt_eval_macro);
-      Diag(getLastFPEvalPragmaLocation(), diag::note_pragma_entered_here);
+    if (getTUFPEvalMethod() ==
+        LangOptions::FPEvalMethodKind::FEM_Indeterminable) {
+      // This is possible if we are in a `fast-math` mode (set either
+      // via a command line option or via a `pragam float_conntrol` or a
+      // `pragma clang fp eval-method`.
+      // __FLT_EVAL_METHOD__ expands to -1.
+      // The `minus` operator is the next token we read from the stream.
+      auto Toks = std::make_unique<Token[]>(2);
+      OS << "-";
+      Tok.setKind(tok::minus);
+      // Push the token `1` to the stream.
+      Token NumberToken;
+      NumberToken.startToken();
+      NumberToken.setKind(tok::numeric_constant);
+      NumberToken.setLiteralData("1");
+      NumberToken.setLength(1);
+      Toks[0] = NumberToken;
+      EnterTokenStream(std::move(Toks), 1, /*DisableMacroExpansion*/ false,
+                       /*IsReinject*/ false);
+    } else {
+      OS << getTUFPEvalMethod();
+      // __FLT_EVAL_METHOD__ expands to a simple numeric value.
+      Tok.setKind(tok::numeric_constant);
+      if (getLastFPEvalPragmaLocation().isValid()) {
+        // The program is ill-formed. The value of __FLT_EVAL_METHOD__ is
+        // altered by the pragma.
+        Diag(Tok, diag::err_illegal_use_of_flt_eval_macro);
+        Diag(getLastFPEvalPragmaLocation(), diag::note_pragma_entered_here);
+      }
     }
   } else if (II == Ident__COUNTER__) {
     // __COUNTER__ expands to a simple numeric value.
Index: clang/include/clang/Basic/DiagnosticParseKinds.td
===================================================================
--- clang/include/clang/Basic/DiagnosticParseKinds.td
+++ clang/include/clang/Basic/DiagnosticParseKinds.td
@@ -1489,7 +1489,8 @@
   "%select{"
   "'fast' or 'on' or 'off'|"
   "'on' or 'off'|"
-  "'ignore', 'maytrap' or 'strict'}2">;
+  "'ignore', 'maytrap' or 'strict'|"
+  "'source' or 'double' or 'extended'}2">;
 
 def err_pragma_invalid_keyword : Error<
   "invalid argument; expected 'enable'%select{|, 'full'}0%select{|, 'assume_safety'}1 or 'disable'">;
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to