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

Leonard Chan via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Thu Nov 15 10:01:21 PST 2018


leonardchan added a comment.

In https://reviews.llvm.org/D53738#1299557, @ebevhan wrote:

> 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.


Good case to bring up. For addition, I think we just need to add an extra condition that checks for unsigned padding in the result. Added this test also.



================
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
----------------
ebevhan wrote:
> What do these comments refer to? The scale is the same for both operands here.
This was meant to be a addition with a short _Accum and _Fract, but accidentally typed this instead. Added that test after this one.


Repository:
  rC Clang

https://reviews.llvm.org/D53738





More information about the cfe-commits mailing list