[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:152
+  case CK_FloatingToIntegral: {
+    llvm::RoundingMode RM = getRoundingMode(CE);
+    Optional<PrimType> ToT = classify(CE->getType());
----------------
According to C standard (6.3.1.4p1):
```
    When a finite value of real floating type is converted to an integer type other than _Bool, the fractional part is discarded (i.e., the value is truncated toward zero).
```
So the conversion should not depend on rounding mode. The same applies to the conversion to boolean (6.3.1.2p1):
```
    When any scalar value is converted to _Bool, the result is 0 if the value compares equal to 0; otherwise, the result is 1.
```



================
Comment at: clang/lib/AST/Interp/ByteCodeExprGen.cpp:617
   case BO_SubAssign:
     if (!this->emitSub(*LT, E))
       return false;
----------------
This and some subsequent operations also should depend on rounding mode?


================
Comment at: clang/lib/AST/Interp/Floating.h:109
+    Floating RHS = B;
+    if (!A.compatibleWith(B))
+      RHS = B.toSemantics(A, RM);
----------------
Do we really need this check? In AST operands of addition always have the same type.


================
Comment at: clang/lib/AST/Interp/Interp.cpp:530-531
+bool CheckFloatResult(InterpState &S, CodePtr OpPC, APFloat::opStatus Status) {
+  // In a constant context, assume that any dynamic rounding mode or FP
+  // exception state matches the default floating-point environment.
+  if (S.inConstantContext())
----------------
This comment is not exact. In a context context rounding mode may be affected by `#pragma STDC FENV_ROUND`.


================
Comment at: clang/lib/AST/Interp/Interp.h:1243
+    // T's bit width.
+    if (!T::canRepresent(Result)) {
+      const Expr *E = S.Current->getExpr(OpPC);
----------------
`Integral::canRepresent` is suitable only for integral-to-intergal conversions. With loss of precision the floating-point numbers can represent wider range of integers.


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

https://reviews.llvm.org/D134859