[PATCH] D62731: [RFC] Add support for options -fp-model= and -fp-speculation= : specify floating point behavior

[Kevin P. Neal via Phabricator via cfe-commits]

kpn added inline comments.


================
Comment at: clang/docs/UsersManual.rst:1307
+
+   ``precise   `` Disables optimizations that are not value-safe on 
+   floating-point data, although FP contraction (FMA) is enabled.
----------------
Extra spaces?


================
Comment at: clang/lib/CodeGen/CodeGenFunction.cpp:126
+  case LangOptions::FPM_Precise:
+  case LangOptions::FPM_Fast:
+    break;
----------------
Wait, so "fast" and "precise" are the same thing? That doesn't sound like where the documentation you put in the ticket says "the compiler preserves the source expression ordering and rounding properties of floating-point".

(Yes, I saw below where "fast" turns on the fast math flags but "precise" doesn't. That doesn't affect my point here.)


================
Comment at: clang/lib/Frontend/CompilerInvocation.cpp:3052
+    else
+      llvm_unreachable("invalid -fp-model setting");
+  }
----------------
Shouldn't this be a call to Diags.Report() like in the code just above it and below? Same question for _some_ other uses of llvm_unreachable().


================
Comment at: clang/test/CodeGen/fpconstrained.c:22
+  // STRICTNOEXCEPT: llvm.experimental.constrained.fadd.f32(float %0, float %1, metadata !"round.dynamic", metadata !"fpexcept.ignore")
+  // PRECISE: fadd float
+  // FAST: fadd fast
----------------
This is another case of "fast" and "precise" doing the same thing. If we're using the regular fadd then it cannot be that "the compiler preserves the source expression ordering and rounding properties of floating-point".


Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D62731/new/

https://reviews.llvm.org/D62731