leonardchan added inline comments.
================
Comment at: clang/lib/CodeGen/CGExprScalar.cpp:141
+ (RHSType->isFixedPointType() &&
+ LHSType->isFixedPointOrIntegerType());
+ }
----------------
ebevhan wrote:
> Can't it just be `LHSType->isFixedPointType() || RHSType->isFixedPointType()`?
>
> I don't think there are cases where one is a fixed-point type and the other
> is not an integer or another fixed-point type, so if one is fixed-point then
> you already know it's a fixed-point operation.
Ah you're right.
================
Comment at: clang/test/Frontend/fixed_point_comparisons.c:56
+
+void TestComparisons() {
+ short _Accum sa;
----------------
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.
Repository:
rC Clang
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D57219/new/
https://reviews.llvm.org/D57219
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
