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

[Serge Pavlov via Phabricator via cfe-commits]

sepavloff added inline comments.


================
Comment at: clang/lib/AST/Interp/ByteCodeExprGen.cpp:607
+    if (*LT == PT_Float) {
+      if (!this->emitAddf(getRoundingMode(E), E))
+        return false;
----------------
As discussed previously, compound assign operator cannot be implemented with the same function as corresponding binary operator in general case.


================
Comment at: clang/lib/AST/Interp/Interp.h:1238
+  auto Status = F.convertToInteger(Result);
+  if constexpr (std::is_same_v<T, Boolean>) {
+    S.Stk.push<T>(T(Result));
----------------
Conversion to boolean can be made with `APFloat::isNonZero`. It would correctly handle all values including NaNs/Infs. `convertToInteger` may leave `Result` unchanged if `Status` is `invalid`.


================
Comment at: clang/lib/AST/Interp/Interp.h:1243
+    // T's bit width.
+    if (!T::canRepresent(Result)) {
+      const Expr *E = S.Current->getExpr(OpPC);
----------------
sepavloff wrote:
> `Integral::canRepresent` is suitable only for integral-to-intergal conversions. With loss of precision the floating-point numbers can represent wider range of integers.
I was wrong, this is float-to-integer conversion. Would it be easier to detect overflow (as well as other cases, like NaN) by checking that `Status==opInvalidOp && F.isFinite()`? 


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

https://reviews.llvm.org/D134859