[Fixed Point Arithmetic] Fixed Point Comparisons

[leonardchan]

Summary

This patch implements fixed point comparisons with other fixed point types and integers. This also provides constant expression evaluation for them.

[ebevhan]

clang/lib/CodeGen/CGExprScalar.cpp

141 ↗

(On Diff #183480)

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.

clang/test/Frontend/fixed_point_comparisons.c

56 ↗

(On Diff #183480)

Missing saturating and saturating/non-saturating comparisons. I'd like to see the differences between unsigned padding and not there, if there are any.

[leonardchan]

clang/lib/CodeGen/CGExprScalar.cpp

141 ↗

(On Diff #183480)

Ah you're right.

clang/test/Frontend/fixed_point_comparisons.c

56 ↗

(On Diff #183480)

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.

[ebevhan]

clang/test/Frontend/fixed_point_comparisons.c

56 ↗

(On Diff #183480)

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?

[leonardchan]

clang/test/Frontend/fixed_point_comparisons.c

56 ↗

(On Diff #183480)

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.

[bjope]

clang/test/Frontend/fixed_point_comparisons.c

2 ↗

(On Diff #183980)

I think it is easier to understand the checks if you for example use UNPADDED/PADDED (or STUDDED/PADDED or something like that) instead of SIGNED/UNSIGNED.

[rjmccall]

clang/lib/CodeGen/CGExprScalar.cpp

3444 ↗

(On Diff #183980)

Are padding bits guaranteed zero or unspecified? Or are we just not really supporting padding bits all the way to IRGen at this time?

[leonardchan]

clang/lib/CodeGen/CGExprScalar.cpp

3444 ↗

(On Diff #183980)

I believe the consensus was leaving them unspecified, so operations that can cause overflow into them would result in undefined behavior.

[rjmccall]

clang/lib/CodeGen/CGExprScalar.cpp

3444 ↗

(On Diff #183980)

If I'm understanding you correctly, you're saying that (1) we are assuming that inputs are zero-padded and (2) we are taking advantage of the non-saturating-overflow-is-UB rule to avoid clearing the padding bits after arithmetic. That's actually what I meant by "guaranteed zero", so we have our labels reversed, but at least we understand each other now.

(I suppose you're thinking of this as "unspecified" because non-saturating arithmetic can leave an unspecified value there. I think of this as a "guarantee" because it's basically a representational invariant: it's a precondition for correctness that the bit is zero, and all operations guarantee that the bit will be zero in their well-defined cases (but overflow is not well-defined). Just a difference in perspective, I guess.)

Is this written down somewhere? Are there targets that use the opposite ABI rule that we might need to support someday?

At any rate, I think it's worth adding a short comment here explaining why it's okay to do a normal comparison despite the possibility of padding bits. Along those lines, is there any value in communicating to the backend that padded-unsigned comparisons could reasonably be done with either a signed or unsigned comparison? Are there interesting targets where one or the other is cheaper?

[leonardchan]

clang/lib/CodeGen/CGExprScalar.cpp

3444 ↗

(On Diff #183980)

Yes. We assume inputs are zero padded, and that non-saturating overflow is undefined so we do not need to clear the padding after writing a new value. Sorry for the misunderstanding. I see what you mean by guaranteed zero.

Overflow in general is controlled by the FX_FRACT_OVERFLOW and FX_ACCUM_OVERFLOW pragmas, where if these are set to DEFAULT, operations that can overflow with these types is undefined according to the standard (4.1.3). In terms of padding bits, clause 6.2.6.3 mentions that the values of padding bits are "unspecified". I imagine this means that we can assume the value to be whatever we want it to, so we can assume that these values are a guaranteed zero.

I believe @ebevhan requested this being added since his downstream implementation used padding to match the scales of signed and unsigned types, so he may be able to offer more information on targets with different ABIs. We don't plan to use any special hardware, so we're following the "typical desktop processor" layout that uses the whole underlying int and no padding (mentioned in Annex 3).

In the same section, the standard also mentions types for other targets that may have padding, so there could be some value in indicating to the backend that for these particular targets, this part of the operand is padding, so select an appropriate operation that performs a comparison on this size type. But I don't know much about these processors and would just be guessing at how they work.

[rjmccall]

clang/lib/CodeGen/CGExprScalar.cpp

3444 ↗

(On Diff #183980)

Okay. If we ever have a target that uses an ABI that (1) includes padding but (2) considers it to be "unspecified" in the sense of "it can validly be non-zero", we'll have to change this code, but I agree it's the right move to not preemptively generalize to cover that possibility.

[ebevhan]

clang/lib/CodeGen/CGExprScalar.cpp

3444 ↗

(On Diff #183980)

Is this written down somewhere? Are there targets that use the opposite ABI rule that we might need to support someday?

What exactly do you mean by 'opposite'? That the padding is cleared after every operation?

It was (and still is) the case in our downstream implementation that all unsigned operations explicitly clear the padding bit after every operation, leading to defined behavior on overflow for such types.

3444 ↗

(On Diff #183980)

To clarify on our use case, we have the padding on the unsigned types mostly to harmonize the behavior of unsigned and signed types. The only real native support is for signed types, but if the unsigned types are padded and then use signed operations to implement them, you get (almost) the same level of performance out of unsigned ones.

The primary difference between the upstream CG and our implementation is that the padding bit is always set to zero after operations. This means that overflow on the unsigned types is essentially defined as modulo wraparound instead of UB, and unsigned types will always contain a valid value.

[ebevhan]

clang/lib/CodeGen/CGExprScalar.cpp

3444 ↗

(On Diff #183980)

Oops, did not mean to submit that comment further up. Was left over from an earlier session.

[rjmccall]

clang/lib/CodeGen/CGExprScalar.cpp

3444 ↗

(On Diff #183980)

That's still effectively the same ABI, just (as you say) intentionally defining the result of non-saturating overflow. I assume you haven't completely eliminated UB and that you still consider an unsigned value with 1 in the padding bit (which you never produce from arithmetic, but which could be produced by e.g. memcpy) to be an illegal representation. What I mean by "opposite" would be an ABI in which that was *not* an illegal representation, and which instead defined the padding bit to be ignored. That ABI would require IRGen to clear the bit before making these comparisons.

[leonardchan]

*ping* @bevinh @rjmccall @bjope Any other comments on this patch?

[rjmccall]

No, this LGTM.

lib/AST/ExprConstant.cpp

if (!EvaluateInteger(E->getRHS(), RHS, Info) || !LHSOK)
       return false;
     if (LHS < RHS)
       return Success(CCR::Less, E);
     if (LHS > RHS)
       return Success(CCR::Greater, E);
     return Success(CCR::Equal, E);
   }
 
+  if (LHSTy->isFixedPointType() || RHSTy->isFixedPointType()) {
+    APFixedPoint LHSFX(Info.Ctx.getFixedPointSemantics(LHSTy));
+    APFixedPoint RHSFX(Info.Ctx.getFixedPointSemantics(RHSTy));
+
+    bool LHSOK = EvaluateFixedPointOrInteger(E->getLHS(), LHSFX, Info);
+    if (!LHSOK && !Info.noteFailure())
+      return false;
+    if (!EvaluateFixedPointOrInteger(E->getRHS(), RHSFX, Info) || !LHSOK)
+      return false;
+    if (LHSFX < RHSFX)
+      return Success(CCR::Less, E);
+    if (LHSFX > RHSFX)
+      return Success(CCR::Greater, E);
+    return Success(CCR::Equal, E);
+  }
+
   if (LHSTy->isAnyComplexType() || RHSTy->isAnyComplexType()) {
     ComplexValue LHS, RHS;
     bool LHSOK;
     if (E->isAssignmentOp()) {
       LValue LV;
       EvaluateLValue(E->getLHS(), LV, Info);
       LHSOK = false;
     } else if (LHSTy->isRealFloatingType()) {

lib/CodeGen/CGExprScalar.cpp

struct BinOpInfo {
   /// Check if the binop can result in a float division by zero.
   bool mayHaveFloatDivisionByZero() const {
     if (isDivremOp())
       if (auto *CFP = dyn_cast<llvm::ConstantFP>(RHS))
         return CFP->isZero();
     return true;
   }
 
-  /// Check if either operand is a fixed point type, in which case, this
+  /// Check if either operand is a fixed point type or integer type, with at
+  /// least one being a fixed point type. In any case, this
   /// operation did not follow usual arithmetic conversion and both operands may
   /// not be the same.
   bool isFixedPointBinOp() const {
-    return isa<BinaryOperator>(E) && Ty->isFixedPointType();
+    // We cannot simply check the result type since comparison operations return
+    // an int.
+    if (const auto *BinOp = dyn_cast<BinaryOperator>(E)) {
+      QualType LHSType = BinOp->getLHS()->getType();
+      QualType RHSType = BinOp->getRHS()->getType();
+      return LHSType->isFixedPointType() || RHSType->isFixedPointType();
+    }
+    return false;
   }
 };
 
 static bool MustVisitNullValue(const Expr *E) {
   // If a null pointer expression's type is the C++0x nullptr_t, then
   // it's not necessarily a simple constant and it must be evaluated
   // for its potential side effects.
   return E->getType()->isNullPtrType();

 
 /// The resulting value must be calculated with exact precision, so the operands
 /// may not be the same type.
 Value *ScalarExprEmitter::EmitFixedPointBinOp(const BinOpInfo &op) {
   using llvm::APSInt;
   using llvm::ConstantInt;
 
   const auto *BinOp = cast<BinaryOperator>(op.E);
-  assert((BinOp->getOpcode() == BO_Add || BinOp->getOpcode() == BO_Sub) &&
-         "Expected operation to be addition or subtraction");
 
   // The result is a fixed point type and at least one of the operands is fixed
   // point while the other is either fixed point or an int. This resulting type
   // should be determined by Sema::handleFixedPointConversions().
   QualType ResultTy = op.Ty;
   QualType LHSTy = BinOp->getLHS()->getType();
   QualType RHSTy = BinOp->getRHS()->getType();
   ASTContext &Ctx = CGF.getContext();
if (ResultFixedSema.isSaturated()) {
                                     ? llvm::Intrinsic::ssub_sat
                                     : llvm::Intrinsic::usub_sat;
       Result = Builder.CreateBinaryIntrinsic(IID, FullLHS, FullRHS);
     } else {
       Result = Builder.CreateSub(FullLHS, FullRHS);
     }
     break;
   }
-  case BO_Mul:
-  case BO_Div:
-  case BO_Shl:
-  case BO_Shr:
-  case BO_Cmp:
   case BO_LT:
+    return CommonFixedSema.isSigned() ? Builder.CreateICmpSLT(FullLHS, FullRHS)
+                                      : Builder.CreateICmpULT(FullLHS, FullRHS);
   case BO_GT:
+    return CommonFixedSema.isSigned() ? Builder.CreateICmpSGT(FullLHS, FullRHS)
+                                      : Builder.CreateICmpUGT(FullLHS, FullRHS);
   case BO_LE:
+    return CommonFixedSema.isSigned() ? Builder.CreateICmpSLE(FullLHS, FullRHS)
+                                      : Builder.CreateICmpULE(FullLHS, FullRHS);
   case BO_GE:
+    return CommonFixedSema.isSigned() ? Builder.CreateICmpSGE(FullLHS, FullRHS)
+                                      : Builder.CreateICmpUGE(FullLHS, FullRHS);
   case BO_EQ:
+    // For equality operations, we assume any padding bits on unsigned types are
+    // zero'd out. They could be overwritten through non-saturating operations
+    // that cause overflow, but this leads to undefined behavior.
+    return Builder.CreateICmpEQ(FullLHS, FullRHS);
   case BO_NE:
+    return Builder.CreateICmpNE(FullLHS, FullRHS);
+  case BO_Mul:
+  case BO_Div:
+  case BO_Shl:
+  case BO_Shr:
+  case BO_Cmp:
   case BO_LAnd:
   case BO_LOr:
   case BO_MulAssign:
   case BO_DivAssign:
   case BO_AddAssign:
   case BO_SubAssign:
   case BO_ShlAssign:
   case BO_ShrAssign:
Value *ScalarExprEmitter::EmitCompare(const BinaryOperator *E,
   if (const MemberPointerType *MPT = LHSTy->getAs<MemberPointerType>()) {
     assert(E->getOpcode() == BO_EQ ||
            E->getOpcode() == BO_NE);
     Value *LHS = CGF.EmitScalarExpr(E->getLHS());
     Value *RHS = CGF.EmitScalarExpr(E->getRHS());
     Result = CGF.CGM.getCXXABI().EmitMemberPointerComparison(
                    CGF, LHS, RHS, MPT, E->getOpcode() == BO_NE);
   } else if (!LHSTy->isAnyComplexType() && !RHSTy->isAnyComplexType()) {
-    Value *LHS = Visit(E->getLHS());
-    Value *RHS = Visit(E->getRHS());
+    BinOpInfo BOInfo = EmitBinOps(E);
+    Value *LHS = BOInfo.LHS;
+    Value *RHS = BOInfo.RHS;
 
     // If AltiVec, the comparison results in a numeric type, so we use
     // intrinsics comparing vectors and giving 0 or 1 as a result
     if (LHSTy->isVectorType() && !E->getType()->isVectorType()) {
       // constants for mapping CR6 register bits to predicate result
       enum { CR6_EQ=0, CR6_EQ_REV, CR6_LT, CR6_LT_REV } CR6;
 
       llvm::Intrinsic::ID ID = llvm::Intrinsic::not_intrinsic;
if (LHSTy->isVectorType() && !E->getType()->isVectorType()) {
       llvm::IntegerType *ResultTy = cast<llvm::IntegerType>(Result->getType());
       if (ResultTy->getBitWidth() > 1 &&
           E->getType() == CGF.getContext().BoolTy)
         Result = Builder.CreateTrunc(Result, Builder.getInt1Ty());
       return EmitScalarConversion(Result, CGF.getContext().BoolTy, E->getType(),
                                   E->getExprLoc());
     }
 
-    if (LHS->getType()->isFPOrFPVectorTy()) {
+    if (BOInfo.isFixedPointBinOp()) {
+      Result = EmitFixedPointBinOp(BOInfo);
+    } else if (LHS->getType()->isFPOrFPVectorTy()) {
       Result = Builder.CreateFCmp(FCmpOpc, LHS, RHS, "cmp");
     } else if (LHSTy->hasSignedIntegerRepresentation()) {
       Result = Builder.CreateICmp(SICmpOpc, LHS, RHS, "cmp");
     } else {
       // Unsigned integers and pointers.
 
       if (CGF.CGM.getCodeGenOpts().StrictVTablePointers &&
           !isa<llvm::ConstantPointerNull>(LHS) &&

test/Frontend/fixed_point_comparisons.c

+// RUN: %clang_cc1 -ffixed-point -triple x86_64-unknown-linux-gnu -S -emit-llvm %s -o - | FileCheck %s --check-prefixes=CHECK,UNPADDED
+// RUN: %clang_cc1 -ffixed-point -triple x86_64-unknown-linux-gnu -fpadding-on-unsigned-fixed-point -S -emit-llvm %s -o - | FileCheck %s --check-prefixes=CHECK,PADDED
+
+// Fixed point against other fixed point
+_Bool b_eq_true = 2.5hk == 2.5uhk;  // CHECK-DAG: @b_eq_true  = {{.*}}global i8 1, align 1
+_Bool b_eq_false = 2.5hk == 2.4uhk; // CHECK-DAG: @b_eq_false = {{.*}}global i8 0, align 1
+
+_Bool b_ne_true = 2.5hk != 2.4uhk;  // CHECK-DAG: @b_ne_true  = {{.*}}global i8 1, align 1
+_Bool b_ne_false = 2.5hk != 2.5uhk; // CHECK-DAG: @b_ne_false = {{.*}}global i8 0, align 1
+
+_Bool b_lt_true = 2.5hk < 2.75uhk; // CHECK-DAG: @b_lt_true  = {{.*}}global i8 1, align 1
+_Bool b_lt_false = 2.5hk < 2.5uhk; // CHECK-DAG: @b_lt_false = {{.*}}global i8 0, align 1
+
+_Bool b_le_true = 2.5hk <= 2.75uhk; // CHECK-DAG: @b_le_true  = {{.*}}global i8 1, align 1
+_Bool b_le_true2 = 2.5hk <= 2.5uhk; // CHECK-DAG: @b_le_true2 = {{.*}}global i8 1, align 1
+_Bool b_le_false = 2.5hk <= 2.4uhk; // CHECK-DAG: @b_le_false = {{.*}}global i8 0, align 1
+
+_Bool b_gt_true = 2.75hk > 2.5uhk;   // CHECK-DAG: @b_gt_true  = {{.*}}global i8 1, align 1
+_Bool b_gt_false = 2.75hk > 2.75uhk; // CHECK-DAG: @b_gt_false = {{.*}}global i8 0, align 1
+
+_Bool b_ge_true = 2.75hk >= 2.5uhk;   // CHECK-DAG: @b_ge_true  = {{.*}}global i8 1, align 1
+_Bool b_ge_true2 = 2.75hk >= 2.75uhk; // CHECK-DAG: @b_ge_true2 = {{.*}}global i8 1, align 1
+_Bool b_ge_false = 2.5hk >= 2.75uhk;  // CHECK-DAG: @b_ge_false = {{.*}}global i8 0, align 1
+
+// Fixed point against int
+_Bool b_ieq_true = 2.0hk == 2;  // CHECK-DAG: @b_ieq_true  = {{.*}}global i8 1, align 1
+_Bool b_ieq_false = 2.0hk == 3; // CHECK-DAG: @b_ieq_false = {{.*}}global i8 0, align 1
+
+_Bool b_ine_true = 2.0hk != 3;  // CHECK-DAG: @b_ine_true  = {{.*}}global i8 1, align 1
+_Bool b_ine_false = 2.0hk != 2; // CHECK-DAG: @b_ine_false = {{.*}}global i8 0, align 1
+
+_Bool b_ilt_true = 2.0hk < 3;  // CHECK-DAG: @b_ilt_true  = {{.*}}global i8 1, align 1
+_Bool b_ilt_false = 2.0hk < 2; // CHECK-DAG: @b_ilt_false = {{.*}}global i8 0, align 1
+
+_Bool b_ile_true = 2.0hk <= 3;  // CHECK-DAG: @b_ile_true  = {{.*}}global i8 1, align 1
+_Bool b_ile_true2 = 2.0hk <= 2; // CHECK-DAG: @b_ile_true2 = {{.*}}global i8 1, align 1
+_Bool b_ile_false = 2.0hk <= 1; // CHECK-DAG: @b_ile_false = {{.*}}global i8 0, align 1
+
+_Bool b_igt_true = 2.0hk > 1;  // CHECK-DAG: @b_igt_true  = {{.*}}global i8 1, align 1
+_Bool b_igt_false = 2.0hk > 2; // CHECK-DAG: @b_igt_false = {{.*}}global i8 0, align 1
+
+_Bool b_ige_true = 2.0hk >= 1;  // CHECK-DAG: @b_ige_true  = {{.*}}global i8 1, align 1
+_Bool b_ige_true2 = 2.0hk >= 2; // CHECK-DAG: @b_ige_true2 = {{.*}}global i8 1, align 1
+_Bool b_ige_false = 2.0hk >= 3; // CHECK-DAG: @b_ige_false = {{.*}}global i8 0, align 1
+
+// Different signage
+// Since we can have different precisions, non powers of 2 fractions may have
+// different actual values when being compared.
+_Bool b_sne_true = 2.6hk != 2.6uhk;
+// UNPADDED-DAG:   @b_sne_true = {{.*}}global i8 1, align 1
+// PADDED-DAG: @b_sne_true = {{.*}}global i8 0, align 1
+
+_Bool b_seq_true = 2.0hk == 2u;  // CHECK-DAG: @b_seq_true  = {{.*}}global i8 1, align 1
+_Bool b_seq_true2 = 2.0uhk == 2; // CHECK-DAG: @b_seq_true2 = {{.*}}global i8 1, align 1
+
+void TestComparisons() {
+  short _Accum sa;
+  _Accum a;
+  unsigned short _Accum usa;
+  unsigned _Accum ua;
+
+  // Each of these should be a fixed point conversion followed by the actual
+  // comparison operation.
+  sa == a;
+  // CHECK:      [[A:%[0-9]+]] = load i16, i16* %sa, align 2
+  // CHECK-NEXT: [[A2:%[0-9]+]] = load i32, i32* %a, align 4
+  // CHECK-NEXT: [[RESIZE_A:%[a-z0-9]+]] = sext i16 [[A]] to i32
+  // CHECK-NEXT: [[UPSCALE_A:%[a-z0-9]+]] = shl i32 [[RESIZE_A]], 8
+  // CHECK-NEXT: {{.*}} = icmp eq i32 [[UPSCALE_A]], [[A2]]
+
+  sa != a;
+  // CHECK:      [[A:%[0-9]+]] = load i16, i16* %sa, align 2
+  // CHECK-NEXT: [[A2:%[0-9]+]] = load i32, i32* %a, align 4
+  // CHECK-NEXT: [[RESIZE_A:%[a-z0-9]+]] = sext i16 [[A]] to i32
+  // CHECK-NEXT: [[UPSCALE_A:%[a-z0-9]+]] = shl i32 [[RESIZE_A]], 8
+  // CHECK-NEXT: {{.*}} = icmp ne i32 [[UPSCALE_A]], [[A2]]
+
+  sa > a;
+  // CHECK:      [[A:%[0-9]+]] = load i16, i16* %sa, align 2
+  // CHECK-NEXT: [[A2:%[0-9]+]] = load i32, i32* %a, align 4
+  // CHECK-NEXT: [[RESIZE_A:%[a-z0-9]+]] = sext i16 [[A]] to i32
+  // CHECK-NEXT: [[UPSCALE_A:%[a-z0-9]+]] = shl i32 [[RESIZE_A]], 8
+  // CHECK-NEXT: {{.*}} = icmp sgt i32 [[UPSCALE_A]], [[A2]]
+
+  sa >= a;
+  // CHECK:      [[A:%[0-9]+]] = load i16, i16* %sa, align 2
+  // CHECK-NEXT: [[A2:%[0-9]+]] = load i32, i32* %a, align 4
+  // CHECK-NEXT: [[RESIZE_A:%[a-z0-9]+]] = sext i16 [[A]] to i32
+  // CHECK-NEXT: [[UPSCALE_A:%[a-z0-9]+]] = shl i32 [[RESIZE_A]], 8
+  // CHECK-NEXT: {{.*}} = icmp sge i32 [[UPSCALE_A]], [[A2]]
+
+  sa < a;
+  // CHECK:      [[A:%[0-9]+]] = load i16, i16* %sa, align 2
+  // CHECK-NEXT: [[A2:%[0-9]+]] = load i32, i32* %a, align 4
+  // CHECK-NEXT: [[RESIZE_A:%[a-z0-9]+]] = sext i16 [[A]] to i32
+  // CHECK-NEXT: [[UPSCALE_A:%[a-z0-9]+]] = shl i32 [[RESIZE_A]], 8
+  // CHECK-NEXT: {{.*}} = icmp slt i32 [[UPSCALE_A]], [[A2]]
+
+  sa <= a;
+  // CHECK:      [[A:%[0-9]+]] = load i16, i16* %sa, align 2
+  // CHECK-NEXT: [[A2:%[0-9]+]] = load i32, i32* %a, align 4
+  // CHECK-NEXT: [[RESIZE_A:%[a-z0-9]+]] = sext i16 [[A]] to i32
+  // CHECK-NEXT: [[UPSCALE_A:%[a-z0-9]+]] = shl i32 [[RESIZE_A]], 8
+  // CHECK-NEXT: {{.*}} = icmp sle i32 [[UPSCALE_A]], [[A2]]
+
+  usa > ua;
+  // CHECK:      [[A:%[0-9]+]] = load i16, i16* %usa, align 2
+  // CHECK-NEXT: [[A2:%[0-9]+]] = load i32, i32* %ua, align 4
+  // CHECK-NEXT: [[RESIZE_A:%[a-z0-9]+]] = zext i16 [[A]] to i32
+  // CHECK-NEXT: [[UPSCALE_A:%[a-z0-9]+]] = shl i32 [[RESIZE_A]], 8
+  // CHECK-NEXT: {{.*}} = icmp ugt i32 [[UPSCALE_A]], [[A2]]
+
+  usa >= ua;
+  // CHECK:      [[A:%[0-9]+]] = load i16, i16* %usa, align 2
+  // CHECK-NEXT: [[A2:%[0-9]+]] = load i32, i32* %ua, align 4
+  // CHECK-NEXT: [[RESIZE_A:%[a-z0-9]+]] = zext i16 [[A]] to i32
+  // CHECK-NEXT: [[UPSCALE_A:%[a-z0-9]+]] = shl i32 [[RESIZE_A]], 8
+  // CHECK-NEXT: {{.*}} = icmp uge i32 [[UPSCALE_A]], [[A2]]
+
+  usa < ua;
+  // CHECK:      [[A:%[0-9]+]] = load i16, i16* %usa, align 2
+  // CHECK-NEXT: [[A2:%[0-9]+]] = load i32, i32* %ua, align 4
+  // CHECK-NEXT: [[RESIZE_A:%[a-z0-9]+]] = zext i16 [[A]] to i32
+  // CHECK-NEXT: [[UPSCALE_A:%[a-z0-9]+]] = shl i32 [[RESIZE_A]], 8
+  // CHECK-NEXT: {{.*}} = icmp ult i32 [[UPSCALE_A]], [[A2]]
+
+  usa <= ua;
+  // CHECK:      [[A:%[0-9]+]] = load i16, i16* %usa, align 2
+  // CHECK-NEXT: [[A2:%[0-9]+]] = load i32, i32* %ua, align 4
+  // CHECK-NEXT: [[RESIZE_A:%[a-z0-9]+]] = zext i16 [[A]] to i32
+  // CHECK-NEXT: [[UPSCALE_A:%[a-z0-9]+]] = shl i32 [[RESIZE_A]], 8
+  // CHECK-NEXT: {{.*}} = icmp ule i32 [[UPSCALE_A]], [[A2]]
+}
+
+void TestIntComparisons() {
+  short _Accum sa;
+  unsigned short _Accum usa;
+
+  int i;
+  unsigned int ui;
+  _Bool b;
+  char c;
+  short s;
+  enum E {
+    A = 2
+  } e;
+
+  // These comparisons shouldn't be that different from comparing against fixed
+  // point types with other fixed point types.
+  sa == i;
+  // CHECK:      [[A:%[0-9]+]] = load i16, i16* %sa, align 2
+  // CHECK-NEXT: [[I:%[0-9]+]] = load i32, i32* %i, align 4
+  // CHECK-NEXT: [[RESIZE_A:%[a-z0-9]+]] = sext i16 [[A]] to i39
+  // CHECK-NEXT: [[RESIZE_I:%[a-z0-9]+]] = sext i32 [[I]] to i39
+  // CHECK-NEXT: [[UPSCALE_I:%[a-z0-9]+]] = shl i39 [[RESIZE_I]], 7
+  // CHECK-NEXT: {{.*}} = icmp eq i39 [[RESIZE_A]], [[UPSCALE_I]]
+
+  sa != i;
+  // CHECK:      [[A:%[0-9]+]] = load i16, i16* %sa, align 2
+  // CHECK-NEXT: [[I:%[0-9]+]] = load i32, i32* %i, align 4
+  // CHECK-NEXT: [[RESIZE_A:%[a-z0-9]+]] = sext i16 [[A]] to i39
+  // CHECK-NEXT: [[RESIZE_I:%[a-z0-9]+]] = sext i32 [[I]] to i39
+  // CHECK-NEXT: [[UPSCALE_I:%[a-z0-9]+]] = shl i39 [[RESIZE_I]], 7
+  // CHECK-NEXT: {{.*}} = icmp ne i39 [[RESIZE_A]], [[UPSCALE_I]]
+
+  sa > i;
+  // CHECK:      [[A:%[0-9]+]] = load i16, i16* %sa, align 2
+  // CHECK-NEXT: [[I:%[0-9]+]] = load i32, i32* %i, align 4
+  // CHECK-NEXT: [[RESIZE_A:%[a-z0-9]+]] = sext i16 [[A]] to i39
+  // CHECK-NEXT: [[RESIZE_I:%[a-z0-9]+]] = sext i32 [[I]] to i39
+  // CHECK-NEXT: [[UPSCALE_I:%[a-z0-9]+]] = shl i39 [[RESIZE_I]], 7
+  // CHECK-NEXT: {{.*}} = icmp sgt i39 [[RESIZE_A]], [[UPSCALE_I]]
+
+  sa >= i;
+  // CHECK:      [[A:%[0-9]+]] = load i16, i16* %sa, align 2
+  // CHECK-NEXT: [[I:%[0-9]+]] = load i32, i32* %i, align 4
+  // CHECK-NEXT: [[RESIZE_A:%[a-z0-9]+]] = sext i16 [[A]] to i39
+  // CHECK-NEXT: [[RESIZE_I:%[a-z0-9]+]] = sext i32 [[I]] to i39
+  // CHECK-NEXT: [[UPSCALE_I:%[a-z0-9]+]] = shl i39 [[RESIZE_I]], 7
+  // CHECK-NEXT: {{.*}} = icmp sge i39 [[RESIZE_A]], [[UPSCALE_I]]
+
+  sa < i;
+  // CHECK:      [[A:%[0-9]+]] = load i16, i16* %sa, align 2
+  // CHECK-NEXT: [[I:%[0-9]+]] = load i32, i32* %i, align 4
+  // CHECK-NEXT: [[RESIZE_A:%[a-z0-9]+]] = sext i16 [[A]] to i39
+  // CHECK-NEXT: [[RESIZE_I:%[a-z0-9]+]] = sext i32 [[I]] to i39
+  // CHECK-NEXT: [[UPSCALE_I:%[a-z0-9]+]] = shl i39 [[RESIZE_I]], 7
+  // CHECK-NEXT: {{.*}} = icmp slt i39 [[RESIZE_A]], [[UPSCALE_I]]
+
+  sa <= i;
+  // CHECK:      [[A:%[0-9]+]] = load i16, i16* %sa, align 2
+  // CHECK-NEXT: [[I:%[0-9]+]] = load i32, i32* %i, align 4
+  // CHECK-NEXT: [[RESIZE_A:%[a-z0-9]+]] = sext i16 [[A]] to i39
+  // CHECK-NEXT: [[RESIZE_I:%[a-z0-9]+]] = sext i32 [[I]] to i39
+  // CHECK-NEXT: [[UPSCALE_I:%[a-z0-9]+]] = shl i39 [[RESIZE_I]], 7
+  // CHECK-NEXT: {{.*}} = icmp sle i39 [[RESIZE_A]], [[UPSCALE_I]]
+
+  usa > ui;
+  // CHECK:      [[A:%[0-9]+]] = load i16, i16* %usa, align 2
+  // CHECK-NEXT: [[I:%[0-9]+]] = load i32, i32* %ui, align 4
+  // UNPADDED-NEXT: [[RESIZE_A:%[a-z0-9]+]] = zext i16 [[A]] to i40
+  // UNPADDED-NEXT: [[RESIZE_I:%[a-z0-9]+]] = zext i32 [[I]] to i40
+  // UNPADDED-NEXT: [[UPSCALE_I:%[a-z0-9]+]] = shl i40 [[RESIZE_I]], 8
+  // UNPADDED-NEXT: {{.*}} = icmp ugt i40 [[RESIZE_A]], [[UPSCALE_I]]
+  // PADDED-NEXT: [[RESIZE_A:%[a-z0-9]+]] = zext i16 [[A]] to i39
+  // PADDED-NEXT: [[RESIZE_I:%[a-z0-9]+]] = zext i32 [[I]] to i39
+  // PADDED-NEXT: [[UPSCALE_I:%[a-z0-9]+]] = shl i39 [[RESIZE_I]], 7
+  // PADDED-NEXT: {{.*}} = icmp ugt i39 [[RESIZE_A]], [[UPSCALE_I]]
+
+  usa >= ui;
+  // CHECK:      [[A:%[0-9]+]] = load i16, i16* %usa, align 2
+  // CHECK-NEXT: [[I:%[0-9]+]] = load i32, i32* %ui, align 4
+  // UNPADDED-NEXT: [[RESIZE_A:%[a-z0-9]+]] = zext i16 [[A]] to i40
+  // UNPADDED-NEXT: [[RESIZE_I:%[a-z0-9]+]] = zext i32 [[I]] to i40
+  // UNPADDED-NEXT: [[UPSCALE_I:%[a-z0-9]+]] = shl i40 [[RESIZE_I]], 8
+  // UNPADDED-NEXT: {{.*}} = icmp uge i40 [[RESIZE_A]], [[UPSCALE_I]]
+  // PADDED-NEXT: [[RESIZE_A:%[a-z0-9]+]] = zext i16 [[A]] to i39
+  // PADDED-NEXT: [[RESIZE_I:%[a-z0-9]+]] = zext i32 [[I]] to i39
+  // PADDED-NEXT: [[UPSCALE_I:%[a-z0-9]+]] = shl i39 [[RESIZE_I]], 7
+  // PADDED-NEXT: {{.*}} = icmp uge i39 [[RESIZE_A]], [[UPSCALE_I]]
+
+  usa < ui;
+  // CHECK:      [[A:%[0-9]+]] = load i16, i16* %usa, align 2
+  // CHECK-NEXT: [[I:%[0-9]+]] = load i32, i32* %ui, align 4
+  // UNPADDED-NEXT: [[RESIZE_A:%[a-z0-9]+]] = zext i16 [[A]] to i40
+  // UNPADDED-NEXT: [[RESIZE_I:%[a-z0-9]+]] = zext i32 [[I]] to i40
+  // UNPADDED-NEXT: [[UPSCALE_I:%[a-z0-9]+]] = shl i40 [[RESIZE_I]], 8
+  // UNPADDED-NEXT: {{.*}} = icmp ult i40 [[RESIZE_A]], [[UPSCALE_I]]
+  // PADDED-NEXT: [[RESIZE_A:%[a-z0-9]+]] = zext i16 [[A]] to i39
+  // PADDED-NEXT: [[RESIZE_I:%[a-z0-9]+]] = zext i32 [[I]] to i39
+  // PADDED-NEXT: [[UPSCALE_I:%[a-z0-9]+]] = shl i39 [[RESIZE_I]], 7
+  // PADDED-NEXT: {{.*}} = icmp ult i39 [[RESIZE_A]], [[UPSCALE_I]]
+
+  usa <= ui;
+  // CHECK:      [[A:%[0-9]+]] = load i16, i16* %usa, align 2
+  // CHECK-NEXT: [[I:%[0-9]+]] = load i32, i32* %ui, align 4
+  // UNPADDED-NEXT: [[RESIZE_A:%[a-z0-9]+]] = zext i16 [[A]] to i40
+  // UNPADDED-NEXT: [[RESIZE_I:%[a-z0-9]+]] = zext i32 [[I]] to i40
+  // UNPADDED-NEXT: [[UPSCALE_I:%[a-z0-9]+]] = shl i40 [[RESIZE_I]], 8
+  // UNPADDED-NEXT: {{.*}} = icmp ule i40 [[RESIZE_A]], [[UPSCALE_I]]
+  // PADDED-NEXT: [[RESIZE_A:%[a-z0-9]+]] = zext i16 [[A]] to i39
+  // PADDED-NEXT: [[RESIZE_I:%[a-z0-9]+]] = zext i32 [[I]] to i39
+  // PADDED-NEXT: [[UPSCALE_I:%[a-z0-9]+]] = shl i39 [[RESIZE_I]], 7
+  // PADDED-NEXT: {{.*}} = icmp ule i39 [[RESIZE_A]], [[UPSCALE_I]]
+
+  // Allow for comparisons with other int like types. These are no different
+  // from comparing to an int other than varying sizes. The integer types are
+  // still converted to ints or unsigned ints from UsualUnaryConversions().
+  sa == b;
+  // CHECK:      [[A:%[0-9]+]] = load i16, i16* %sa, align 2
+  // CHECK-NEXT: [[B:%[0-9]+]] = load i8, i8* %b, align 1
+  // CHECK-NEXT: %tobool = trunc i8 [[B]] to i1
+  // CHECK-NEXT: [[CONV_B:%[a-z0-9]+]] = zext i1 %tobool to i32
+  // CHECK-NEXT: [[RESIZE_A:%[a-z0-9]+]] = sext i16 [[A]] to i39
+  // CHECK-NEXT: [[RESIZE_B:%[a-z0-9]+]] = sext i32 [[CONV_B]] to i39
+  // CHECK-NEXT: [[UPSCALE_B:%[a-z0-9]+]] = shl i39 [[RESIZE_B]], 7
+  // CHECK-NEXT: {{.*}} = icmp eq i39 [[RESIZE_A]], [[UPSCALE_B]]
+
+  sa == c;
+  // CHECK:      [[A:%[0-9]+]] = load i16, i16* %sa, align 2
+  // CHECK-NEXT: [[C:%[0-9]+]] = load i8, i8* %c, align 1
+  // CHECK-NEXT: [[CONV_C:%[a-z0-9]+]] = sext i8 [[C]] to i32
+  // CHECK-NEXT: [[RESIZE_A:%[a-z0-9]+]] = sext i16 [[A]] to i39
+  // CHECK-NEXT: [[RESIZE_C:%[a-z0-9]+]] = sext i32 [[CONV_C]] to i39
+  // CHECK-NEXT: [[UPSCALE_C:%[a-z0-9]+]] = shl i39 [[RESIZE_C]], 7
+  // CHECK-NEXT: {{.*}} = icmp eq i39 [[RESIZE_A]], [[UPSCALE_C]]
+
+  sa == s;
+  // CHECK:      [[A:%[0-9]+]] = load i16, i16* %sa, align 2
+  // CHECK-NEXT: [[S:%[0-9]+]] = load i16, i16* %s, align 2
+  // CHECK-NEXT: [[CONV_S:%[a-z0-9]+]] = sext i16 [[S]] to i32
+  // CHECK-NEXT: [[RESIZE_A:%[a-z0-9]+]] = sext i16 [[A]] to i39
+  // CHECK-NEXT: [[RESIZE_S:%[a-z0-9]+]] = sext i32 [[CONV_S]] to i39
+  // CHECK-NEXT: [[UPSCALE_S:%[a-z0-9]+]] = shl i39 [[RESIZE_S]], 7
+  // CHECK-NEXT: {{.*}} = icmp eq i39 [[RESIZE_A]], [[UPSCALE_S]]
+
+  // An enum value is IntegralCast to an unsigned int.
+  usa == e;
+  // CHECK:      [[A:%[0-9]+]] = load i16, i16* %usa, align 2
+  // CHECK-NEXT: [[I:%[0-9]+]] = load i32, i32* %e, align 4
+  // UNPADDED-NEXT: [[RESIZE_A:%[a-z0-9]+]] = zext i16 [[A]] to i40
+  // UNPADDED-NEXT: [[RESIZE_I:%[a-z0-9]+]] = zext i32 [[I]] to i40
+  // UNPADDED-NEXT: [[UPSCALE_I:%[a-z0-9]+]] = shl i40 [[RESIZE_I]], 8
+  // UNPADDED-NEXT: {{.*}} = icmp eq i40 [[RESIZE_A]], [[UPSCALE_I]]
+  // PADDED-NEXT: [[RESIZE_A:%[a-z0-9]+]] = zext i16 [[A]] to i39
+  // PADDED-NEXT: [[RESIZE_I:%[a-z0-9]+]] = zext i32 [[I]] to i39
+  // PADDED-NEXT: [[UPSCALE_I:%[a-z0-9]+]] = shl i39 [[RESIZE_I]], 7
+  // PADDED-NEXT: {{.*}} = icmp eq i39 [[RESIZE_A]], [[UPSCALE_I]]
+}
+
+void TestComparisonSignage() {
+  short _Accum sa;
+  unsigned short _Accum usa;
+  int i;
+  unsigned int ui;
+
+  // Signed vs unsigned fixed point comparison
+  sa == usa;
+  // CHECK:      [[A:%[0-9]+]] = load i16, i16* %sa, align 2
+  // CHECK-NEXT: [[A2:%[0-9]+]] = load i16, i16* %usa, align 2
+  // UNPADDED-NEXT: [[RESIZE_A:%[a-z0-9]+]] = sext i16 [[A]] to i17
+  // UNPADDED-NEXT: [[UPSCALE_A:%[a-z0-9]+]] = shl i17 [[RESIZE_A]], 1
+  // UNPADDED-NEXT: [[RESIZE_A2:%[a-z0-9]+]] = zext i16 [[A2]] to i17
+  // UNPADDED-NEXT: {{.*}} = icmp eq i17 [[UPSCALE_A]], [[RESIZE_A2]]
+  // PADDED-NEXT: {{.*}} = icmp eq i16 [[A]], [[A2]]
+
+  // Signed int vs unsigned fixed point
+  sa == ui;
+  // CHECK:      [[A:%[0-9]+]] = load i16, i16* %sa, align 2
+  // CHECK-NEXT: [[I:%[0-9]+]] = load i32, i32* %ui, align 4
+  // CHECK-NEXT: [[RESIZE_A:%[a-z0-9]+]] = sext i16 [[A]] to i40
+  // CHECK-NEXT: [[RESIZE_I:%[a-z0-9]+]] = zext i32 [[I]] to i40
+  // CHECK-NEXT: [[UPSCALE_I:%[a-z0-9]+]] = shl i40 [[RESIZE_I]], 7
+  // CHECK-NEXT: {{.*}} = icmp eq i40 [[RESIZE_A]], [[UPSCALE_I]]
+
+  // Signed fixed point vs unsigned int
+  usa == i;
+  // CHECK:      [[A:%[0-9]+]] = load i16, i16* %usa, align 2
+  // CHECK-NEXT: [[I:%[0-9]+]] = load i32, i32* %i, align 4
+  // UNPADDED-NEXT: [[RESIZE_A:%[a-z0-9]+]] = zext i16 [[A]] to i40
+  // UNPADDED-NEXT: [[RESIZE_I:%[a-z0-9]+]] = sext i32 [[I]] to i40
+  // UNPADDED-NEXT: [[UPSCALE_I:%[a-z0-9]+]] = shl i40 [[RESIZE_I]], 8
+  // UNPADDED-NEXT: {{.*}} = icmp eq i40 [[RESIZE_A]], [[UPSCALE_I]]
+  // PADDED-NEXT: [[RESIZE_A:%[a-z0-9]+]] = zext i16 [[A]] to i39
+  // PADDED-NEXT: [[RESIZE_I:%[a-z0-9]+]] = sext i32 [[I]] to i39
+  // PADDED-NEXT: [[UPSCALE_I:%[a-z0-9]+]] = shl i39 [[RESIZE_I]], 7
+  // PADDED-NEXT: {{.*}} = icmp eq i39 [[RESIZE_A]], [[UPSCALE_I]]
+}
+
+void TestSaturationComparisons() {
+  short _Accum sa;
+  _Accum a;
+  _Sat short _Accum sat_sa;
+  _Sat _Accum sat_a;
+  _Sat unsigned short _Accum sat_usa;
+
+  // These are effectively the same as conversions with their non-saturating
+  // counterparts since when comparing, we convert both operands to a common
+  // type that should be able to hold both values.
+  sat_sa == sat_a;
+  // CHECK:      [[A:%[0-9]+]] = load i16, i16* %sat_sa, align 2
+  // CHECK-NEXT: [[A2:%[0-9]+]] = load i32, i32* %sat_a, align 4
+  // CHECK-NEXT: [[RESIZE_A:%[a-z0-9]+]] = sext i16 [[A]] to i32
+  // CHECK-NEXT: [[UPSCALE_A:%[a-z0-9]+]] = shl i32 [[RESIZE_A]], 8
+  // CHECK-NEXT: {{.*}} = icmp eq i32 [[UPSCALE_A]], [[A2]]
+
+  sat_sa == a;
+  // CHECK:      [[A:%[0-9]+]] = load i16, i16* %sat_sa, align 2
+  // CHECK-NEXT: [[A2:%[0-9]+]] = load i32, i32* %a, align 4
+  // CHECK-NEXT: [[RESIZE_A:%[a-z0-9]+]] = sext i16 [[A]] to i32
+  // CHECK-NEXT: [[UPSCALE_A:%[a-z0-9]+]] = shl i32 [[RESIZE_A]], 8
+  // CHECK-NEXT: {{.*}} = icmp eq i32 [[UPSCALE_A]], [[A2]]
+
+  sat_sa == sat_usa;
+  // CHECK:      [[A:%[0-9]+]] = load i16, i16* %sat_sa, align 2
+  // CHECK-NEXT: [[A2:%[0-9]+]] = load i16, i16* %sat_usa, align 2
+  // UNPADDED-NEXT: [[RESIZE_A:%[a-z0-9]+]] = sext i16 [[A]] to i17
+  // UNPADDED-NEXT: [[UPSCALE_A:%[a-z0-9]+]] = shl i17 [[RESIZE_A]], 1
+  // UNPADDED-NEXT: [[RESIZE_A2:%[a-z0-9]+]] = zext i16 [[A2]] to i17
+  // UNPADDED-NEXT: {{.*}} = icmp eq i17 [[UPSCALE_A]], [[RESIZE_A2]]
+  // PADDED-NEXT: {{.*}} = icmp eq i16 [[A]], [[A2]]
+}
+
+void StoreBooleanResult() {
+  short _Accum sa;
+  _Accum a;
+  int res;
+
+  // Check that the result can properly be stored as an int.
+  res = sa == a;
+  // CHECK:      [[A:%[0-9]+]] = load i16, i16* %sa, align 2
+  // CHECK-NEXT: [[A2:%[0-9]+]] = load i32, i32* %a, align 4
+  // CHECK-NEXT: [[RESIZE_A:%[a-z0-9]+]] = sext i16 [[A]] to i32
+  // CHECK-NEXT: [[UPSCALE_A:%[a-z0-9]+]] = shl i32 [[RESIZE_A]], 8
+  // CHECK-NEXT: [[RES:%[0-9]+]] = icmp eq i32 [[UPSCALE_A]], [[A2]]
+  // CHECK-NEXT: %conv = zext i1 [[RES]] to i32
+  // CHECK-NEXT: store i32 %conv, i32* %res, align 4
+}