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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
