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

[Andy Kaylor via Phabricator via cfe-commits]

andrew.w.kaylor added inline comments.


================
Comment at: clang/include/clang/Basic/DiagnosticDriverKinds.td:444
 
+def warn_drv_experimental_fp_control_incomplete_opt : Warning<
+  "Support for floating point control option %0 is incomplete and experimental">,
----------------
rupprecht wrote:
> andrew.w.kaylor wrote:
> > rupprecht wrote:
> > > mibintc wrote:
> > > > @kpn thought it would be a good idea to add a Warning that the implementation of float control is experimental and partially implemented.  That's what this is for.
> > > Instead of adding a warning, I'd like to propose `-frounding-math` not be enabled unless an additional flag (e.g. `-fexperimental-float-control`) is passed. Or maybe this feature should be called `-f[no-]experimental-rounding-math` instead.
> > > 
> > > There are plenty of builds that are already specifying `-frounding-math` (e.g. they also support building w/ a compiler such as gcc that implements this), and adding this experimental/incomplete implementation is a surprise to those builds.
> > > 
> > > If I'm wrong and it's completely safe to ignore the warning (maybe it's incomplete but not in any unsafe way), we should just not have it at all.
> > You raise an interesting point about people who might be using -frounding-math already. However, if they are using this flag because they also sometimes build with a compiler such as gcc that supports the flag, they are currently getting incorrect behavior from clang. Without this patch, clang silently ignores the option and the optimizer silently ignores the fact that the program may be changing the rounding mode dynamically. The user may or may not be aware of this.
> > 
> > With this patch such a user is likely to observe two effects: (1) their code will suddenly get slower, and (2) it will probably start behaving correctly with regard to rounding mode changes. The rounding behavior will definitely not get worse. I think the warning is useful as an indication that something has changed. I don't think requiring an additional option should be necessary.
> > However, if they are using this flag because they also sometimes build with a compiler such as gcc that supports the flag, they are currently getting incorrect behavior from clang
> 
> Incorrect, yes, but also likely valid behavior. "experimental" seems to imply a miscompile when using this option should not be unexpected.
> 
> As I suggested before: if I'm wrong, and this behavior is only going to make the code more correct (as you suggest), can we remove the warning that this must be ack'd explicitly by adding `-Wno-experimental-float-control` to builds? I don't understand the motivation for the warning.
The "experimental" code won't be incorrect in any way that the code generated when we ignore the option is. The things that have been implemented will work correctly. The things that are not implemented will have the potential to disregard runtime changes to the rounding mode. Currently, dynamic changes to the rounding mode always have the potential of being ignored.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62731