[PATCH] D46915: [Fixed Point Arithmetic] Fixed Point Precision Bits and Fixed Point Literals

[Leonard Chan via Phabricator via cfe-commits]

leonardchan added inline comments.


================
Comment at: lib/AST/ExprConstant.cpp:9323
+      if (Value.isSigned() && Value.isMinSignedValue() && E->canOverflow() &&
+          !HandleOverflow(Info, E, -Value.extend(Value.getBitWidth() + 1),
+                          E->getType()))
----------------
ebevhan wrote:
> Is signed fixed-point overflow actually considered undefined behavior? I'm not familiar with what Embedded-C says about this.
> 
> Also, this routine will likely print the wrong number for fixed-point values.
I believe clause 4.1.3 states that if `FX_FRACT_OVERFLOW` or `FX_ACCUM_OVERFLOW` are set to `SAT` then overflow behavior on the respective types is saturation. Otherwise, overflow has undefined behavior by default or if the pragma is set to `DEFAULT`.


================
Comment at: lib/Sema/SemaExpr.cpp:3407
+
+    if (Literal.fixedPointType == FPT_ACCUM) {
+      if (isSigned) {
----------------
ebevhan wrote:
> It should be possible to do this in a much more concise manner. In our port we do this with a table lookup indexed by the type selection bits.
Replaced with getters for corresponding type followed by getters for information about the bits.


================
Comment at: lib/Sema/SemaExpr.cpp:3483
+      assert(
+          (fbits + ibits + 1 <= bit_width) &&
+          "The total fractional, integral, and sign bits exceed the bit width");
----------------
ebevhan wrote:
> It does not feel like verifying this is the responsibility of the literal parser.
Right, this is taken care of in the target creation.


Repository:
  rC Clang

https://reviews.llvm.org/D46915