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



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to