ebevhan added inline comments.
================
Comment at: clang/lib/AST/ExprConstant.cpp:7568
+static bool EvaluateFixedPoint(const Expr *E, APValue &Result, EvalInfo &Info)
{
+ if (E->getType()->isFixedPointType()) {
----------------
This could provide an APFixedPoint rather than APValue.
================
Comment at: clang/lib/AST/ExprConstant.cpp:7582
+
+static bool EvaluateFixedPointOrInteger(const Expr *E, APValue &Result,
+ EvalInfo &Info) {
----------------
Technically, this will always produce an APFixedPoint so it doesn't really need
to return APValue either.
================
Comment at: clang/lib/AST/ExprConstant.cpp:7584
+ EvalInfo &Info) {
+ auto FXSema = Info.Ctx.getFixedPointSemantics(E->getType());
+ if (E->getType()->isIntegerType()) {
----------------
The semantic is not used in the fixed-point path.
================
Comment at: clang/lib/AST/ExprConstant.cpp:9927
}
return Success(-Value, E);
}
----------------
I think I've mentioned this before, but just as a reminder; this doesn't
perform correctly for saturation.
================
Comment at: clang/lib/AST/ExprConstant.cpp:9979
+ APFixedPoint Result =
+ LHSFX.getFixedPoint().add(RHSFX.getFixedPoint()).convert(ResultFXSema);
+ return Success(Result, E);
----------------
Perhaps this should call HandleOverflow (or do something else) to signal that
overflow occurred. HandleOverflow only seems to emit a note, though, but I
think it should probably be a warning.
Maybe for casts as well? Might get double warnings then, though.
================
Comment at: clang/lib/Basic/FixedPoint.cpp:25
bool Upscaling = DstScale > getScale();
+ bool Overflowed = false;
----------------
I think it's a bit cleaner if you avoid this variable and simply assign
`Overflow` directly here, with the 'else' cases below replaced with `else if
(Overflow)`.
That style isn't cleaner in `add` if you use the APInt add_ov functions though,
so maybe it's better to keep it this way for consistency.
================
Comment at: clang/lib/Basic/FixedPoint.cpp:170
+ Result = ThisVal + OtherVal;
+ Overflowed = ThisVal.isSigned() ? Result.slt(ThisVal) :
Result.ult(ThisVal);
+ }
----------------
You could use the add_ov functions here.
Repository:
rC Clang
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D55868/new/
https://reviews.llvm.org/D55868
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
