[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;
+
----------------
ebevhan wrote:
> 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.
Although.,. I guess I see your point in that an intermediate result of k+e technically "does not fit within the range of the fixed-point type"... but I wonder if treating such cases as overflow is particularly meaningful. I don't find there to be much of a distinction between such a case and the case where the exact result lands inbetween two representable values. We just end up with a less precise result.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73186