[PATCH] D53738: [Fixed Point Arithmetic] Fixed Point Addition

[John McCall via Phabricator via cfe-commits]

rjmccall added inline comments.


================
Comment at: clang/lib/Sema/SemaExpr.cpp:1304
+    RHSTy = ResultTy;
+  }
+
----------------
ebevhan wrote:
> rjmccall wrote:
> > Hmm.  So adding a signed integer to an unsigned fixed-point type always converts the integer to unsigned?  That's a little weird, but only because the fixed-point rule seems to be the other way.  Anyway, I assume it's not a bug in the spec.
> `handleFixedPointConversion` only calculates the result type of the expression, not the calculation type. The final result type of the operation will be the unsigned fixed-point type, but the calculation itself will be done in a signed type with enough precision to represent both the signed integer and the unsigned fixed-point type. 
> 
> Though, if the result ends up being negative, the final result is undefined unless the type is saturating. I don't think there is a test for the saturating case (signed integer + unsigned saturating fixed-point) in the SaturatedAddition tests.
> `handleFixedPointConversion` only calculates the result type of the expression, not the calculation type.

Right, I understand that, but the result type of the expression obviously impacts the expressive range of the result, since you can end up with a negative value.

Hmm.  With that said, if the general principle is to perform the operation with full precision on the original operand values, why are unsigned fixed-point operands coerced to their corresponding signed types *before* the operation?  This is precision-limiting if the unsigned representation doesn't use a padding bit.  That seems like a bug in the spec.


Repository:
  rC Clang

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

https://reviews.llvm.org/D53738