[PATCH] D53738: [Fixed Point Arithmetic] Fixed Point Addition

[Leonard Chan via Phabricator via cfe-commits]

leonardchan added inline comments.


================
Comment at: clang/lib/CodeGen/CGExprScalar.cpp:3223-3247
+    if (ResultWidth < CommonWidth) {
+      // In the event we extended the sign of both operands, the intrinsic will
+      // not saturate to the initial bit width of the result type. In this case,
+      // we can default to use min/max clamping. This can arise from adding an
+      // operand of an int type whose width is larger than the width of the
+      // other fixed point operand.
+      // If we end up implementing the intrinsics for saturating ints to
----------------
There is a case when we cannot use the sadd/uadd intrinsics because those intrinsics saturate to the width of the operands when instead we want to saturate to the width of the resulting fixed point type. The problem is that if we are given operands with different scales that require extending then shifting before adding, the bit width no longer matches that of the resulting saturated type, so the sat intrinsics instead saturate to the extended width instead of the result type width. This could be a problem with my logic though and there could be a better way to implement addition.

Otherwise, as far as I can tell, this could be addressed by replacing the clamping with an intrinsic, which would create a use case for the signed/unsigned saturation intrinsics. Alternatively, the addition intrinsics could be expanded to also provide another argument that contains a desired saturation bit width, which would allow for collapsing the saturation type case into a call to this one intrinsic function. 



Repository:
  rC Clang

https://reviews.llvm.org/D53738