ebevhan added inline comments.

================
Comment at: clang/lib/CodeGen/CGExprScalar.cpp:3385
+  if (IsCommonSaturated)
+    CommonFixedSema.setSaturated(false);
+
----------------
ebevhan wrote:
> Can EmitFixedPointConversion not determine this on its own?
What I meant here was that rather than zeroing out the saturation mode on the 
common semantic because we know that the common conversion won't saturate, 
EmitFixedPointConversion should be able to pick up on the fact that converting 
from semantic A to semantic B shouldn't cause saturation and not emit a 
saturating conversion in that case. Then you can set the saturation on the 
common semantic properly, since the operation should be saturating.

Even for something like `sat_uf * sat_uf` with padding, where the common type 
conversion should be `(16w, 15scale, uns, sat, pad) -> (15w, 15scale, uns, sat, 
nopad)`, EmitFixedPointConversion shouldn't emit a saturation, since the number 
of integral bits hasn't changed.


================
Comment at: clang/test/Frontend/fixed_point_add.c:269
+  // UNSIGNED-NEXT: [[SUM:%[0-9]+]] = call i15 @llvm.uadd.sat.i15(i15 
[[USA_TRUNC]], i15 [[USA_SAT_TRUNC]])
+  // UNSIGNED-NEXT: [[SUM_EXT:%[a-z0-9]+]] = zext i15 [[SUM]] to i16
+  // UNSIGNED-NEXT: store i16 [[SUM_EXT]], i16* %usa_sat, align 2
----------------
This is probably a candidate for an isel optimization. This operation also 
works as an `i16 ssat.add` with a negative-clamp-to-zero afterwards, and if the 
target supports `i16 ssat.add` natively then it will likely be a lot more 
efficient than whatever an `i15 uadd.sat` produces.


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