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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
