[PATCH] D62730: [RFC] Alternate implementation of D53157 IRBuilder for Constrained FP using enumeration vs MDNode and add support for fp-model and fp-speculation language options

[Melanie Blower via Phabricator via cfe-commits]

mibintc marked 3 inline comments as done.
mibintc added inline comments.


================
Comment at: include/llvm/IR/FPState.h:9-31
+  enum FPModelKind {
+    FPM_Off,
+    FPM_Precise,
+    FPM_Strict,
+    FPM_Fast
+  };
+
----------------
kpn wrote:
> lebedev.ri wrote:
> > All this needs comments, and possibly better names.
> > `FPM_Off`,etc is very non-self-explanatory.
> Shouldn't there be an "accept default" value? That way CreateConstrainedFAdd()/CreateConstrainedBinOp() can accept enumerations and avoid having front ends having to deal with MDNode*.
By "off" I just meant, not specified, and therefore the default setting would prevail.  If you prefer "default" instead of "off" that's fine by me. 


================
Comment at: include/llvm/IR/IRBuilder.h:247
+    switch (DefaultConstrainedExcept) {
+    case FPState::CE_Off:
+    case FPState::CE_Strict:
----------------
In order to match the code that @kpn wrote, I should set DefaultConstrainedExcept = strict instead of just leaving it off.


================
Comment at: include/llvm/IR/IRBuilder.h:261
+    switch (DefaultConstrainedRounding) {
+    case FPState::CR_Off:
+    case FPState::CR_Dynamic:
----------------
to match @kpn patch, I should set DefaultConstrainedRounding to dynamic instead of leaving it off


Repository:
  rL LLVM

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

https://reviews.llvm.org/D62730