[PATCH] D73186: [AST] Add fixed-point multiplication constant evaluation.

[Bevin Hansson via Phabricator via cfe-commits]

ebevhan added inline comments.


================
Comment at: clang/lib/Basic/FixedPoint.cpp:242
+  } else
+    Overflowed = Result < Min || Result > Max;
+
----------------
rjmccall wrote:
> If the maximum expressible value is *k*, and the fully-precise multiplication yields *k+e* for some epsilon *e* that isn't representable in the result semantics, is that considered an overflow?  If so, I think you need to do the shift after these bound checks, since the shift destroys the difference between *k* and *k+e*.  That is, unless there's a compelling mathematical argument that it's not possible to overflow only in the fully-precision multiplication — but while I think that's possibly true of `_Fract` (since *k^2 < k*), it seems unlikely to be true of `_Accum`, although I haven't looked for a counter-example.  And if there is a compelling argument, it should probably be at least alluded to in a comment.
> 
> Would this algorithm be simpler if you took advantage of the fact that `APFixedPointSemantics` doesn't have to correspond to a real type?  You could probably just convert to a double-width common semantics, right?
> If the maximum expressible value is *k*, and the fully-precise multiplication yields *k+e* for some epsilon *e* that isn't representable in the result semantics, is that considered an overflow? If so, I think you need to do the shift after these bound checks, since the shift destroys the difference between *k* and *k+e*.

I don't think I would consider that to be overflow; that's precision loss. E-C considers these to be different:

> If the source value cannot be represented exactly by the fixed-point type, the source value is rounded to either the closest fixed-point value greater than the source value (rounded up) or to the closest fixed-point value less than the source value (rounded down).
>
> When the source value does not fit within the range of the fixed-point type, the conversion overflows. [...]
>
> [...]
>
> If the result type of an arithmetic operation is a fixed-point type, [...] the calculated result is the mathematically exact result with overflow handling and rounding performed to the full precision of the result type as explained in 4.1.3. 

There is also no value of `e` that would affect saturation. Any full precision calculation that gives `k+e` must be `k` after downscaling, since the bits that represent `e` must come from the extra precision range. Even though `k+e` is technically larger than `k`, saturation would still just give us `k` after truncating out `e`, so the end result is the same.

> Would this algorithm be simpler if you took advantage of the fact that APFixedPointSemantics doesn't have to correspond to a real type? You could probably just convert to a double-width common semantics, right?

It's likely possible to use APFixedPoint in the calculations here, but I used APInt to make the behavior explicit and not accidentally be dependent on the behavior of APFixedPoint's conversions or operations.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73186