ebevhan added a comment.

I'd be interested in seeing tests for two saturating unsigned _Fract with and 
without padding.

If the padding case emits a uadd_sat, that seems wrong; uadd_sat doesn't 
saturate on the padding bit, but will saturate the whole number, which can 
result in invalid representation both with or without saturation taking effect.

Common semantics for unsigned types with padding might need a bit of trickery 
to get it to do the right thing.



================
Comment at: clang/lib/CodeGen/CGExprScalar.cpp:3383
+
+  // onvert the operands to the full precision type.
+  Value *FullLHS = EmitFixedPointConversion(LHS, LHSFixedSema, CommonFixedSema,
----------------
Missing a C there.


================
Comment at: clang/lib/Sema/SemaExpr.cpp:1306
+    OtherTy = ResultTy;
+  }
+
----------------
leonardchan wrote:
> ebevhan wrote:
> > Does this handle compound assignment? The other functions handle this 
> > specially.
> Currently no. I was initially intending to add addition compound assignment 
> in a different patch, but do you think it would be appropriate to include it 
> here? I imagine the changes to Sema wouldn't be difficult, but would probably 
> require adding a lot more tests. 
That's fine by me, then. Take it in the next patch.


================
Comment at: clang/test/Frontend/fixed_point_add.c:36
+
+  // To smaller scale and same width
+  // CHECK:      [[SA:%[0-9]+]] = load i16, i16* %sa, align 2
----------------
What do these comments refer to? The scale is the same for both operands here.


Repository:
  rC Clang

https://reviews.llvm.org/D53738



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

Reply via email to