[PATCH] D134859: [clang][Interp] Implement basic support for floating point values

[Joshua Cranmer via Phabricator via cfe-commits]

jcranmer-intel added a comment.

Some extra constexpr tests that may be interesting:

- Constant expressions that produce floating point exceptions other than `FE_INEXACT`.
- Subnormal values as operands, as well as operations with normal operands that produce denormal values (i.e., check for DAZ/FTZ, respectively.)
- qNaN arithmetic
- sNaN arithmetic
- different NaN payloads (this can also catch out differences caused by host hardware: there's disagreement as to which NaN payload is used in an fadd, some processors prefer the first one, others prefer the second one).
- converting infinity, NaN, numbers outside INT_MAX range to integers
- converting integers to floating-point outside the latter's range--that's only possible with `unsigned __int128` and `float` or `uint32_t` and `half` though

Some of these tests relate to things that you haven't implemented yet--and that's fine--but it's worth keeping in mind for future patches.



================
Comment at: clang/lib/AST/Interp/Floating.h:27-29
+  template <unsigned ReprBits> struct Repr;
+  template <> struct Repr<32> { using Type = float; };
+  template <> struct Repr<64> { using Type = double; };
----------------
aaron.ballman wrote:
> Er, how will this extend to `long double` where the number of bits is rather more difficult?
Or `half` and `bfloat`, which are both 16-bit floating-point types?


================
Comment at: clang/lib/AST/Interp/Floating.h:31-33
+  // The primitive representing the Floating.
+  using ReprT = typename Repr<Bits>::Type;
+  ReprT V;
----------------
My gut reaction is that, for floating-point types, you're probably better off using `APFloat` directly rather than trying to use the float/double platform types directly. Directly using platform types for computation means you potentially have changes based on the how clang itself was compiled--especially if someone decides to compile clang with `-ffast-math` and now all your constexpr stuff is executing in FTZ/DAZ mode. `APFloat` may be slower, but it also provides more protection from floating-point chicanery.


================
Comment at: clang/lib/AST/Interp/Floating.h:82
+  ComparisonCategoryResult compare(const Floating &RHS) const {
+    return Compare(V, RHS.V);
+  }
----------------
This is supposed to be capable of returning unordered if `*this` or `RHS` is a NaN, right?


================
Comment at: clang/lib/AST/Interp/Floating.h:106
+  // -------
+
+  static bool add(Floating A, Floating B, unsigned OpBits, Floating *R) {
----------------
tbaeder wrote:
> The operations here don't do overflow or UB handling.
They also don't support rounding mode. (constexpr evaluation arguably should at least support `FENV_ROUND`)


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

https://reviews.llvm.org/D134859