[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.

Yes, thanks, looks a lot better.  Just a few tweaks now.



================
Comment at: clang/include/clang/Basic/LangOptions.h:348
   VersionTuple getOpenCLVersionTuple() const;
+
 };
----------------
Spurious change.


================
Comment at: clang/include/clang/Driver/Options.td:1152
+def frounding_math : Flag<["-"], "frounding-math">, Group<f_Group>, Flags<[CC1Option]>;
+def fno_rounding_math : Flag<["-"], "fno-rounding-math">, Group<f_Group>, Flags<[CC1Option]>;
 def ftrapping_math : Flag<["-"], "ftrapping-math">, Group<f_Group>, Flags<[CC1Option]>;
----------------
It looks like both of these can now be written with `BooleanFFlag`.


================
Comment at: clang/lib/CodeGen/CodeGenFunction.cpp:148
+    llvm_unreachable("Unsupported FP Exception Behavior");
+  }
+
----------------
Please make functions that do these translations, and please make them use exhaustive switches with `llvm_unreachable` at the end.


================
Comment at: clang/test/Driver/clang_f_opts.c:323
 // RUN: -fprofile-values                                                      \
-// RUN: -frounding-math                                                       \
 // RUN: -fschedule-insns                                                      \
----------------
Looks like the intent of this test is that you pull this to the lines above, to test that we don't emit an error on it.  You should also test `-ffp-model`.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D62731