rupprecht 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">, ---------------- 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. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D62731/new/ https://reviews.llvm.org/D62731 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
