leonardchan added inline comments.
================
Comment at: clang/test/Frontend/fixed_point_comparisons.c:56
+
+void TestComparisons() {
+ short _Accum sa;
----------------
ebevhan wrote:
> leonardchan wrote:
> > ebevhan wrote:
> > > Missing saturating and saturating/non-saturating comparisons. I'd like to
> > > see the differences between unsigned padding and not there, if there are
> > > any.
> > I don't think there should be since we compare by converting to a common
> > type that should fit both operands, but it does help to have tests that
> > also confirm this. Added some saturation cases under
> > `TestSaturationComparisons`.
> >
> > As for padding, `TestSaturationComparisons` have cases comparing signed
> > with unsigned types, and there are other cases in `TestComparisons` and
> > `TestIntComparisons` that deal with unsigned comparisons. The way the lit
> > tests are organized, lines marked with `CHECK` mean that those lines are
> > produced for both the padding and no padding cases whereas `SIGNED` or
> > `UNSIGNED` are produced exclusively for no padding and padding cases,
> > respectively.
> There is a difference between saturating signed types and saturating unsigned
> types, though; the common type of two of the same saturating unsigned type is
> one bit less due to padding.
>
> Unless there is something in the type commoning that I've missed?
You're right. It's just that when comparing values, we care about padding and
not saturation since we aren't "writing" a value, so a comparison between a
`_Sat short _Accum` and `_Sat unsigned short _Accum` should be the same as one
for a `short _Accum` and `unsigned short _Accum`. With unsigned padding, both
operations should not result in any resizing/scaling since they're already the
same size and scale.
Added a test case for this under `TestSaturationComparisons`.
Repository:
rC Clang
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D57219/new/
https://reviews.llvm.org/D57219
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
