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

[John McCall via Phabricator via cfe-commits]

rjmccall added inline comments.


================
Comment at: clang/lib/Basic/FixedPoint.cpp:242
+  } else
+    Overflowed = Result < Min || Result > Max;
+
----------------
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?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73186