ebevhan added a comment.
I think I've already expressed that I'm not at all a fan of encoding the
full-precision logic in the operations themselves and not explicitly in the
AST. We already have (well, not yet, but we will) routines for emitting
conversions to/from/between fixed-point types of arbitrary semantics - why not
use that instead of reimplementing the same logic for every binary operation?
As it is now, EmitFixedPointAdd replicates the logic for both converting from
integer to fixed-point and fixed-point to fixed-point. You mention a special
case with saturating addition, but this special case only exists because the
routine needs to do fixed-point->saturating fixed-point on its own. If all
EmitFixedPointAdd did was perform an add between two fixed-point values with
the same semantics, then the logic would be much simpler and the act of
conversion would be delegated to the routines that actually handle it.
I want to float the idea again of adding an AST type to encapsulate an
arbitrary fixed-point semantic and using that as the 'common type' for binary
operations involving fixed-point types. This would enable
UsualArithmeticConversions to handle fixed-point types similarly to how it does
today (find the 'common' full precision type, implicitly cast the LHS and RHS
if necessary). There is one new thing that it would have to do; the result
after the operation should not be the full precision type, but rather be casted
to the operand type of highest rank. I don't think the existing code in
SemaExpr can handle the case of adding an implicit cast after the operation. I
don't think it should be hard to add, though.
================
Comment at: clang/lib/Sema/SemaExpr.cpp:1393
+/// rules in N1169 4.1.4.
+QualType Sema::FixedPointConversions(ExprResult &FixedExpr,
+ ExprResult &OtherExpr, bool IsCompAssign)
{
----------------
I'm not sure I understand why this is in a separate function. What part of this
doesn't work in UsualArithmeticConversions, in a 'handleFixedPointConversion'
similar to the other cases?
================
Comment at: clang/lib/Sema/SemaExpr.cpp:1416
+ // converted to its corresponding signed fixed-point type and the resulting
+ // type is the type of the converted operand.
+ if (OtherTy->isSignedFixedPointType() &&
----------------
I feel like this logic can be folded into the rank handling. Does it work
properly if you give signed types higher rank than unsigned ones?
================
Comment at: clang/lib/Sema/SemaExpr.cpp:1435
+ // account rounding and overflow) to the precision of the resulting type.
+ if (OtherTy->isIntegerType())
+ return FixedTy;
----------------
This can be avoided by making all integer types lower rank than the fixed point
types. The rank between them doesn't matter, either; they can all be the same.
Repository:
rC Clang
https://reviews.llvm.org/D53738
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
