[PATCH] D62731: Add support for options -frounding-math, ftrapping-math, -fp-model=, and -fp-exception-behavior=, : Specify floating point behavior

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

Thanks.  A few things about the functionality parts of the patch now.



================
Comment at: clang/include/clang/Basic/CodeGenOptions.def:238
 CODEGENOPT(UnsafeFPMath      , 1, 0) ///< Allow unsafe floating point optzns.
+CODEGENOPT(RoundingFPMath    , 1, 0) ///< Rounding floating point optzns.
 CODEGENOPT(UnwindTables      , 1, 0) ///< Emit unwind tables.
----------------
Why do we need both a code-gen option and a language option?


================
Comment at: clang/include/clang/Basic/LangOptions.h:366
+      FPEB = Value;
+    }
+
----------------
Everything here is a "setting", and in the context of this type they're all FP.  Please name these methods something like `getRoundingMode()`.

Does this structure really need to exist as opposed to tracking the dimensions separately?  Don't we already track some of this somewhere?  We should subsume that state into these values rather than tracking them separately.


================
Comment at: clang/lib/CodeGen/CodeGenFunction.cpp:108
+void CodeGenFunction::SetFPModel(void)
+{
+  auto fpRoundingMode = getLangOpts().getFPMOptions().getFPRoundingModeSetting();
----------------
Code style: please use `()` instead of `(void)`, and please place open-braces on the same line as the declaration.


================
Comment at: clang/lib/CodeGen/CodeGenFunction.h:4158
+  /// SetFPModel - Control floating point behavior via fp-model settings.
+  void SetFPModel(void);
+
----------------
Don't use `(void)`, please.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D62731