[Fixed Point Arithmetic] Validation Test for Fixed Point Binary Operations and Saturated Addition

[leonardchan]

Summary

This patch contains tests for validating the logic behind each builtin operation on fixed point types and tests on addition between saturated _Fract types.

• Detecting overflow involves checking if the sign bit flipped or if the first bit in the padding gets flipped.
• More macros wer added to the FixedPoint.h header on the min and max values for each type which will be required for operations on saturated types.
• Updated the logic when converting between fixed point types to take into account saturation. Fixed point type conversions do not fall under the "usual arithmetic conversions" where the resulting type on a binary operation resulting in a fixed point type does not need to be the type of either operands.
• Rounded down _Fract literals of 1 (1.0hr, 1.0r, 1.0lr) to the respective maximum values for each _Fract type.

[leonardchan]

• formatting
• Running lli threw a segfault in the test, though this was probably because it was using whatever hist jit was available to optimize the code instead of just interpreting it. Forcing it just interpret fixes this.

[ebevhan]

I cannot say that I'm pleased with the CodeGen emission of the operations as pure IR. I can only assume that you do not have hardware specifically tailored for these operations, as matching this type of code ought to be quite difficult after optimization is performed.

include/clang/AST/Type.h

6591

If these need to be passed a ASTContext anyway, why not have these functions on ASTContext to begin with?

include/clang/Basic/FixedPoint.h.in

35

As mentioned in other patches, these should not be macros (this entire file should probably be removed altogether).

lib/CodeGen/CGExprScalar.cpp

3151

All of these values can clearly be calculated based on the scaling factor and the width of the type.

3206

Factor out conversion between fixed-point types into its own function so it can be reused for other cases, such as for other operations and actual conversions. It should probably not take QualTypes to convert to but rather arbitrary widths and scales, so it can be used to upscale to/downscale from 'intermediate' common calculation types.

3211

I would much rather see these operations emitted as intrinsics rather than straight IR... but I know that wasn't part of your proposal.

lib/Sema/SemaExpr.cpp

1264

This is incorrect. The resulting type of a binary operation is absolutely one of the two operands. However, the calculation might be done as a type that is not one of the two, as it must be done in the full (combined) precision of both operands.

That is a detail reserved for CodeGen, however.

3551

Should these changes not be part of the patch that adds the literal parsing code?

[ebevhan]

Also, this patch and all of the following 'Validation Test' patches do much more than just add tests, they add plenty of functionality as well. In general, it's quite difficult to tell which patches add what.

I think it would be much clearer if the patches that claim to add tests only add tests.

include/clang/AST/Type.h

 
 // Return the number of integral bits in a fixed point type.
 unsigned getFixedPointIBits(const Type &Ty);
 
 inline unsigned getFixedPointIBits(const QualType &Ty) {
   return getFixedPointIBits(*Ty);
 }
 
+// For a given fixed point type, if the type is unsaturated,
+// return the saturated equivalent of it. Otherwise if it is
+// saturated, return back the type itself.
+QualType getCorrespondingSaturatedFixedPointType(ASTContext &Context,
+                                                 const Type &Ty);
+
+// For a given fixed point type, if the type is unsigned,
+// return the signed equivalent of it. Otherwise if it is
+// signed, return back the type itself.
+QualType getCorrespondingSignedFixedPointType(ASTContext &Context,
+                                              const Type &Ty);
+
 } // namespace clang
 
 #endif // LLVM_CLANG_AST_TYPE_H

include/clang/Basic/FixedPoint.h.in

 #ifndef LLVM_CLANG_BASIC_FIXEDPOINT_H
 #define LLVM_CLANG_BASIC_FIXEDPOINT_H
 
+#include <climits>
+
+/************ Bit widths ******************/
+
 // Fractional bits of _Accum types
 #define BUILTIN_SACCUM_FBIT     @SACCUM_FBIT@
 #define BUILTIN_ACCUM_FBIT      @ACCUM_FBIT@
 #define BUILTIN_LACCUM_FBIT     @LACCUM_FBIT@
 #define BUILTIN_USACCUM_FBIT    @USACCUM_FBIT@
 #define BUILTIN_UACCUM_FBIT     @UACCUM_FBIT@
 #define BUILTIN_ULACCUM_FBIT    @ULACCUM_FBIT@
 
 // Fractional bits of _Fract types
 #define BUILTIN_SFRACT_FBIT     @SFRACT_FBIT@
 #define BUILTIN_FRACT_FBIT      @FRACT_FBIT@
 #define BUILTIN_LFRACT_FBIT     @LFRACT_FBIT@
 #define BUILTIN_USFRACT_FBIT    @USFRACT_FBIT@
 #define BUILTIN_UFRACT_FBIT     @UFRACT_FBIT@
 #define BUILTIN_ULFRACT_FBIT    @ULFRACT_FBIT@
 
 // Integral bits of _Accum types
 #define BUILTIN_SACCUM_IBIT     @SACCUM_IBIT@
 #define BUILTIN_ACCUM_IBIT      @ACCUM_IBIT@
 #define BUILTIN_LACCUM_IBIT     @LACCUM_IBIT@
 #define BUILTIN_USACCUM_IBIT    @USACCUM_IBIT@
 #define BUILTIN_UACCUM_IBIT     @UACCUM_IBIT@
 #define BUILTIN_ULACCUM_IBIT    @ULACCUM_IBIT@
 
+/************ Max/min vals ******************/
+
+// Max values of each _Accum type as integer bytes
+#define SACCUM_MAX_AS_INT       ((1ULL << (BUILTIN_SACCUM_FBIT + BUILTIN_SACCUM_IBIT)) - 1)
+#define ACCUM_MAX_AS_INT        ((1ULL << (BUILTIN_ACCUM_FBIT + BUILTIN_ACCUM_IBIT)) - 1)
+#define LACCUM_MAX_AS_INT       ((1ULL << (BUILTIN_LACCUM_FBIT + BUILTIN_LACCUM_IBIT)) - 1)
+#define USACCUM_MAX_AS_INT      ((1ULL << (BUILTIN_USACCUM_FBIT + BUILTIN_USACCUM_IBIT)) - 1)
+#define UACCUM_MAX_AS_INT       ((1ULL << (BUILTIN_UACCUM_FBIT + BUILTIN_UACCUM_IBIT)) - 1)
+#define ULACCUM_MAX_AS_INT      ((static_cast<__int128>(1ULL) << (BUILTIN_ULACCUM_FBIT + BUILTIN_ULACCUM_IBIT)) - 1)
+
+// Max values of each _Fract type as integer bytes
+#define SFRACT_MAX_AS_INT       ((1ULL << BUILTIN_SFRACT_FBIT) - 1)
+#define FRACT_MAX_AS_INT        ((1ULL << BUILTIN_FRACT_FBIT) - 1)
+#define LFRACT_MAX_AS_INT       ((1ULL << BUILTIN_LFRACT_FBIT) - 1)
+#define USFRACT_MAX_AS_INT      ((1ULL << BUILTIN_USFRACT_FBIT) - 1)
+#define UFRACT_MAX_AS_INT       ((1ULL << BUILTIN_UFRACT_FBIT) - 1)
+#define ULFRACT_MAX_AS_INT      ((1ULL << BUILTIN_ULFRACT_FBIT) - 1)
+
+// Min values of each _Accum type as integer bytes
+#define SACCUM_MIN_AS_INT       (1ULL << (BUILTIN_SACCUM_FBIT + BUILTIN_SACCUM_IBIT))
+#define ACCUM_MIN_AS_INT        (1ULL << (BUILTIN_ACCUM_FBIT + BUILTIN_ACCUM_IBIT))
+#define LACCUM_MIN_AS_INT       (1ULL << (BUILTIN_LACCUM_FBIT + BUILTIN_LACCUM_IBIT))
+#define USACCUM_MIN_AS_INT      0ULL
+#define UACCUM_MIN_AS_INT       0ULL
+#define ULACCUM_MIN_AS_INT      0ULL
+
+// Min values of each _Fract type as integer bytes
+#define SFRACT_MIN_AS_INT       (1ULL << BUILTIN_SFRACT_FBIT)
+#define FRACT_MIN_AS_INT        (1ULL << BUILTIN_FRACT_FBIT)
+#define LFRACT_MIN_AS_INT       (1ULL << BUILTIN_LFRACT_FBIT)
+#define USFRACT_MIN_AS_INT      0ULL
+#define UFRACT_MIN_AS_INT       0ULL
+#define ULFRACT_MIN_AS_INT      0ULL
+
+/************ Fixed Point Bitmasks ******************/
+
+// Used to capture only the relevant bits when comparing with other types
+
+// _Accum bitmasks
+#define SACCUM_MASK             ((1ULL << (BUILTIN_SACCUM_FBIT + BUILTIN_SACCUM_IBIT + 1)) - 1)
+#define ACCUM_MASK              ((1ULL << (BUILTIN_ACCUM_FBIT + BUILTIN_ACCUM_IBIT + 1)) - 1)
+#define LACCUM_MASK             ((1ULL << (BUILTIN_LACCUM_FBIT + BUILTIN_LACCUM_IBIT + 1)) - 1)
+#define USACCUM_MASK            ((1ULL << (BUILTIN_USACCUM_FBIT + BUILTIN_USACCUM_IBIT)) - 1)
+#define UACCUM_MASK             ((1ULL << (BUILTIN_UACCUM_FBIT + BUILTIN_UACCUM_IBIT)) - 1)
+#define ULACCUM_MASK            ((1ULL << (BUILTIN_ULACCUM_FBIT + BUILTIN_ULACCUM_IBIT)) - 1)
+
+// _Fract bitmasks
+#define SFRACT_MASK             ((1ULL << (BUILTIN_SFRACT_FBIT + 1)) - 1)
+#define FRACT_MASK              ((1ULL << (BUILTIN_FRACT_FBIT + 1)) - 1)
+#define LFRACT_MASK             ((1ULL << (BUILTIN_LFRACT_FBIT + 1)) - 1)
+#define USFRACT_MASK            ((1ULL << BUILTIN_USFRACT_FBIT) - 1)
+#define UFRACT_MASK             ((1ULL << BUILTIN_UFRACT_FBIT) - 1)
+#define ULFRACT_MASK            ((1ULL << BUILTIN_ULFRACT_FBIT) - 1)
+
 #endif

lib/AST/Type.cpp

switch (BT->getKind()) {
     case BuiltinType::SatFract:
     case BuiltinType::SatLongFract:
     case BuiltinType::SatUShortFract:
     case BuiltinType::SatUFract:
     case BuiltinType::SatULongFract:
       return 0;
   }
 }
+
+// For a given fixed point type, if the type is unsaturated,
+// return the saturated equivalent of it. Otherwise if it is
+// saturated, return back the type itself.
+QualType clang::getCorrespondingSaturatedFixedPointType(ASTContext &Context,
+                                                        const Type &Ty) {
+  assert(Ty.isFixedPointType());
+
+  const auto &BT = Ty.getAs<BuiltinType>();
+  switch (BT->getKind()) {
+    default:
+      llvm_unreachable("Not a fixed point type!");
+    case BuiltinType::ShortAccum:
+    case BuiltinType::SatShortAccum:
+      return Context.SatShortAccumTy;
+
+    case BuiltinType::Accum:
+    case BuiltinType::SatAccum:
+      return Context.SatAccumTy;
+
+    case BuiltinType::LongAccum:
+    case BuiltinType::SatLongAccum:
+      return Context.SatLongAccumTy;
+
+    case BuiltinType::UShortAccum:
+    case BuiltinType::SatUShortAccum:
+      return Context.SatUnsignedShortAccumTy;
+
+    case BuiltinType::UAccum:
+    case BuiltinType::SatUAccum:
+      return Context.SatUnsignedAccumTy;
+
+    case BuiltinType::ULongAccum:
+    case BuiltinType::SatULongAccum:
+      return Context.SatUnsignedLongAccumTy;
+
+    case BuiltinType::ShortFract:
+    case BuiltinType::SatShortFract:
+      return Context.SatShortFractTy;
+
+    case BuiltinType::Fract:
+    case BuiltinType::SatFract:
+      return Context.SatFractTy;
+
+    case BuiltinType::LongFract:
+    case BuiltinType::SatLongFract:
+      return Context.SatLongFractTy;
+
+    case BuiltinType::UShortFract:
+    case BuiltinType::SatUShortFract:
+      return Context.SatUnsignedShortFractTy;
+
+    case BuiltinType::UFract:
+    case BuiltinType::SatUFract:
+      return Context.SatUnsignedFractTy;
+
+    case BuiltinType::ULongFract:
+    case BuiltinType::SatULongFract:
+      return Context.SatUnsignedLongFractTy;
+  }
+}
+
+// For a given fixed point type, if the type is unsigned,
+// return the signed equivalent of it. Otherwise if it is
+// signed, return back the type itself.
+QualType clang::getCorrespondingSignedFixedPointType(ASTContext &Context,
+                                                     const Type &Ty) {
+  assert(Ty.isFixedPointType());
+
+  const auto &BT = Ty.getAs<BuiltinType>();
+  switch (BT->getKind()) {
+    default:
+      llvm_unreachable("Not a fixed point type!");
+    case BuiltinType::ShortAccum:
+    case BuiltinType::UShortAccum:
+      return Context.ShortAccumTy;
+
+    case BuiltinType::Accum:
+    case BuiltinType::UAccum:
+      return Context.AccumTy;
+
+    case BuiltinType::LongAccum:
+    case BuiltinType::ULongAccum:
+      return Context.LongAccumTy;
+
+    case BuiltinType::SatShortAccum:
+    case BuiltinType::SatUShortAccum:
+      return Context.SatShortAccumTy;
+
+    case BuiltinType::SatAccum:
+    case BuiltinType::SatUAccum:
+      return Context.SatAccumTy;
+
+    case BuiltinType::SatLongAccum:
+    case BuiltinType::SatULongAccum:
+      return Context.SatLongAccumTy;
+
+    case BuiltinType::ShortFract:
+    case BuiltinType::UShortFract:
+      return Context.ShortFractTy;
+
+    case BuiltinType::Fract:
+    case BuiltinType::UFract:
+      return Context.FractTy;
+
+    case BuiltinType::LongFract:
+    case BuiltinType::ULongFract:
+      return Context.LongFractTy;
+
+    case BuiltinType::SatShortFract:
+    case BuiltinType::SatUShortFract:
+      return Context.SatShortFractTy;
+
+    case BuiltinType::SatFract:
+    case BuiltinType::SatUFract:
+      return Context.SatFractTy;
+
+    case BuiltinType::SatLongFract:
+    case BuiltinType::SatULongFract:
+      return Context.SatLongFractTy;
+  }
+}

lib/CodeGen/CGExprScalar.cpp

if (op.LHS->getType()->isFPOrFPVectorTy()) {
     // Try to form an fmuladd.
     if (Value *FMulAdd = tryEmitFMulAdd(op, CGF, Builder))
       return FMulAdd;
 
     Value *V = Builder.CreateFAdd(op.LHS, op.RHS, "add");
     return propagateFMFlags(V, op);
   }
 
+  if (op.Ty->isSaturatedFixedPointType()) {
+    llvm::Type *opTy = CGF.CGM.getTypes().ConvertType(op.Ty);
+
+    assert(op.LHS->getType() == op.RHS->getType());
+    assert(op.LHS->getType() == opTy);
+
+    llvm::Value *SatMaxVal;
+    llvm::Value *SatMinVal;
+
+    const auto &BT = op.Ty->getAs<BuiltinType>();
+    switch (BT->getKind()) {
+      default:
+        llvm_unreachable("Unhandled saturated signed fixed point type");
+      case BuiltinType::SatShortAccum:
+        SatMaxVal = llvm::ConstantInt::get(opTy, SACCUM_MAX_AS_INT);
+        SatMinVal = llvm::ConstantInt::get(opTy, SACCUM_MIN_AS_INT);
+        break;
+      case BuiltinType::SatAccum:
+        SatMaxVal = llvm::ConstantInt::get(opTy, ACCUM_MAX_AS_INT);
+        SatMinVal = llvm::ConstantInt::get(opTy, ACCUM_MIN_AS_INT);
+        break;
+      case BuiltinType::SatLongAccum:
+        SatMaxVal = llvm::ConstantInt::get(opTy, LACCUM_MAX_AS_INT);
+        SatMinVal = llvm::ConstantInt::get(opTy, LACCUM_MIN_AS_INT);
+        break;
+      case BuiltinType::SatUShortAccum:
+        SatMaxVal = llvm::ConstantInt::get(opTy, USACCUM_MAX_AS_INT);
+        SatMinVal = llvm::ConstantInt::get(opTy, USACCUM_MIN_AS_INT);
+        break;
+      case BuiltinType::SatUAccum:
+        SatMaxVal = llvm::ConstantInt::get(opTy, UACCUM_MAX_AS_INT);
+        SatMinVal = llvm::ConstantInt::get(opTy, UACCUM_MIN_AS_INT);
+        break;
+      case BuiltinType::SatULongAccum:
+        SatMaxVal = llvm::ConstantInt::get(opTy, ULACCUM_MAX_AS_INT);
+        SatMinVal = llvm::ConstantInt::get(opTy, ULACCUM_MIN_AS_INT);
+        break;
+      case BuiltinType::SatShortFract:
+        SatMaxVal = llvm::ConstantInt::get(opTy, SFRACT_MAX_AS_INT);
+        SatMinVal = llvm::ConstantInt::get(opTy, SFRACT_MIN_AS_INT);
+        break;
+      case BuiltinType::SatFract:
+        SatMaxVal = llvm::ConstantInt::get(opTy, FRACT_MAX_AS_INT);
+        SatMinVal = llvm::ConstantInt::get(opTy, FRACT_MIN_AS_INT);
+        break;
+      case BuiltinType::SatLongFract:
+        SatMaxVal = llvm::ConstantInt::get(opTy, LFRACT_MAX_AS_INT);
+        SatMinVal = llvm::ConstantInt::get(opTy, LFRACT_MIN_AS_INT);
+        break;
+      case BuiltinType::SatUShortFract:
+        SatMaxVal = llvm::ConstantInt::get(opTy, USFRACT_MAX_AS_INT);
+        SatMinVal = llvm::ConstantInt::get(opTy, USFRACT_MIN_AS_INT);
+        break;
+      case BuiltinType::SatUFract:
+        SatMaxVal = llvm::ConstantInt::get(opTy, UFRACT_MAX_AS_INT);
+        SatMinVal = llvm::ConstantInt::get(opTy, UFRACT_MIN_AS_INT);
+        break;
+      case BuiltinType::SatULongFract:
+        SatMaxVal = llvm::ConstantInt::get(opTy, ULFRACT_MAX_AS_INT);
+        SatMinVal = llvm::ConstantInt::get(opTy, ULFRACT_MIN_AS_INT);
+        break;
+    }
+
+    unsigned MSBBitShift;
+    if (op.Ty->isSignedFixedPointType()) {
+      MSBBitShift = getFixedPointIBits(op.Ty) + getFixedPointFBits(op.Ty);
+    } else {
+      MSBBitShift = getFixedPointIBits(op.Ty) + getFixedPointFBits(op.Ty) - 1;
+    }
+
+    llvm::Value *Sum = Builder.CreateAdd(op.LHS, op.RHS);
+    llvm::Value *LHSMSB = Builder.CreateLShr(op.LHS, MSBBitShift);
+    llvm::Value *RHSMSB = Builder.CreateLShr(op.RHS, MSBBitShift);
+    llvm::Value *ResultMSB = Builder.CreateLShr(Sum, MSBBitShift);
+
+    if (op.Ty->isSignedFixedPointType()) {
+      // Cap at max if both operand signs were 0 and the result sign is 1
+      llvm::Value *UseSatMax = Builder.CreateAnd(
+          Builder.CreateNot(Builder.CreateOr(LHSMSB, RHSMSB)), ResultMSB);
+      UseSatMax = Builder.CreateIntCast(
+          UseSatMax, llvm::Type::getInt1Ty(ResultMSB->getContext()),
+          /*isSigned=*/true);
+
+      // Cap at min if both operand signs were 1 and the result sign is 0
+      llvm::Value *UseSatMin = Builder.CreateAnd(
+          Builder.CreateAnd(LHSMSB, RHSMSB), Builder.CreateNot(ResultMSB));
+      UseSatMin = Builder.CreateIntCast(
+          UseSatMin, llvm::Type::getInt1Ty(ResultMSB->getContext()),
+          /*isSigned=*/true);
+
+      return Builder.CreateSelect(
+          UseSatMax, SatMaxVal,
+          Builder.CreateSelect(UseSatMin, SatMinVal, Sum));
+    } else {
+      // Cap at max if the resulting MSB is less than either operand MSB
+      llvm::Value *UseSatMax = Builder.CreateAnd(
+          Builder.CreateOr(LHSMSB, RHSMSB), Builder.CreateNot(ResultMSB));
+      UseSatMax = Builder.CreateIntCast(
+          UseSatMax, llvm::Type::getInt1Ty(ResultMSB->getContext()),
+          /*isSigned=*/true);
+      return Builder.CreateSelect(UseSatMax, SatMaxVal, Sum);
+    }
+  }
+
   return Builder.CreateAdd(op.LHS, op.RHS, "add");
 }
 
 Value *ScalarExprEmitter::EmitSub(const BinOpInfo &op) {
   // The LHS is always a pointer if either side is.
   if (!op.LHS->getType()->isPointerTy()) {
     if (op.Ty->isSignedIntegerOrEnumerationType()) {
       switch (CGF.getLangOpts().getSignedOverflowBehavior()) {

lib/Sema/SemaExpr.cpp

static QualType handleIntToFixedPointConversion(Sema &S,
   assert(IntTy->isIntegerType());
   assert(FixedPointTy->isFixedPointType());
 
   IntExpr =
       S.ImpCastExprToType(IntExpr.get(), FixedPointTy, CK_IntegralToFixedPoint);
   return FixedPointTy;
 }
 
-/// \brief Handle arithmethic conversion with fixed point types.  Helper
-/// function of UsualArithmeticConversions().
+/// \brief Handle arithmethic conversion with fixed point types. The usual
+/// arithmetic conversions do not apply to fixed point type conversions between
+/// integers or other fixed point types due to potential loss of precision.
+/// For this case of fixed point types, the resulting type in a binary operation
+/// does not need to be exactly one of the 2 operand types.
+/// Implemented according to Clause 6.3.1.8 of ISO/IEC JTC1 SC22 WG14 N1169.
 static QualType handleFixedPointConversion(Sema &S, ExprResult &LHS,
                                            ExprResult &RHS, QualType LHSType,
                                            QualType RHSType,
                                            bool IsCompAssign) {
   // At this point, the only other type we should be able to convert to is
   // integer types since all prior types were handled beforehand in
   // UsualArithmeticConventions().
   bool LHSFixed = LHSType->isFixedPointType();
   bool RHSFixed = RHSType->isFixedPointType();
 
   if (LHSFixed && RHSFixed) {
-    // Cast up the smaller operand to the bigger
-    int order = S.Context.getFixedPointTypeOrder(LHSType, RHSType);
-    if (order > 0) {
-      RHS = S.ImpCastExprToType(RHS.get(), LHSType, CK_FixedPointCast);
-      return LHSType;
-    } else if (!order) {
-      bool LHSSigned = LHSType->isSignedFixedPointType();
-      bool RHSSigned = RHSType->isSignedFixedPointType();
-      if (LHSSigned && !RHSSigned) {
-        RHS = S.ImpCastExprToType(RHS.get(), LHSType, CK_FixedPointCast);
-        return LHSType;
-      } else if (!LHSSigned && RHSSigned) {
-        if (!IsCompAssign)
-          LHS = S.ImpCastExprToType(LHS.get(), RHSType, CK_FixedPointCast);
-        return RHSType;
-      } else {
-        assert(LHSType == RHSType);
-        return LHSType;
-      }
-    } else {
-      if (!IsCompAssign)
-        LHS = S.ImpCastExprToType(LHS.get(), RHSType, CK_FixedPointCast);
-      return RHSType;
-    }
+    bool LHSSat = LHSType->isSaturatedFixedPointType();
+    bool RHSSat = RHSType->isSaturatedFixedPointType();
+    int order = S.Context.getFixedPointTypeOrder(LHSType, RHSType);
+
+    QualType result = (order >= 0) ? LHSType : RHSType;
+    if (LHSSigned || RHSSigned)
+      result = getCorrespondingSignedFixedPointType(S.Context, *result);
+    if (LHSSat || RHSSat)
+      result = getCorrespondingSaturatedFixedPointType(S.Context, *result);
+
+    if (LHSType != result && !IsCompAssign)
+      LHS = S.ImpCastExprToType(LHS.get(), result, CK_FixedPointCast);
+
+    if (RHSType != result)
+      RHS = S.ImpCastExprToType(RHS.get(), result, CK_FixedPointCast);
+
+    return result;
   } else if (LHSFixed) {
     assert(RHSType->isIntegerType());
     return handleIntToFixedPointConversion(S, LHS, RHS, LHSType, RHSType);
   } else if (RHSFixed) {
     assert(LHSType->isIntegerType());
     return handleIntToFixedPointConversion(S, RHS, LHS, RHSType, LHSType);
   } else {
     llvm_unreachable(
if (Literal.isFixedPointLiteral()) {
 
     // TODO: Check sizes to make sure we don't overflow or fit a value that
     // can't fit into the number of bits we have.
     double float_val = Val.convertToDouble();
     assert(float_val >= 0);
     double int_part;
     double fract_part = modf(float_val, &int_part);
     uint64_t int_part_as_int = static_cast<uint64_t>(int_part);
+    uint64_t fract_part_as_int =
+        static_cast<uint64_t>(fract_part * (1ULL << fbits));
+    uint64_t final_fixed_point_as_int =
+        (int_part_as_int << fbits) + fract_part_as_int;
 
-    if (Literal.fixedPointType == FPT_FRACT && int_part_as_int) {
+    if (Literal.fixedPointType == FPT_FRACT) {
+      if (float_val > 1) {
         Diag(Tok.getLocation(), diag::err_integral_part_on_fract);
-    } else {
+      } else if (int_part_as_int == 1) {
+        // Represent 1.0r as the max possible value for this _Fract type
+        final_fixed_point_as_int = (1ULL << fbits) - 1;
+      }
+    } else if (Literal.fixedPointType == FPT_ACCUM) {
       // Make sure the integral part fits into the integral bits we have.
       uint64_t max_int_val = 0;
       if (isSigned) {
         max_int_val = 1ULL << (ibits - 1);  // min signed is -2^(n-1)
       } else {
         max_int_val = (1ULL << ibits) - 1;
       }
 
       // TODO: What should be done for literals with no unsigned suffix whose
       // value is equal to the absolute value of the smallest possible value for
       // that type. Ie. 128.0hk -> -128.0hk? What happens is user writes
       // "-128.0hk" since we are only interpretting a literal and do now know of
       // the unary operator?
       if (int_part_as_int > max_int_val) {
         Diag(Tok.getLocation(), diag::err_integral_part_of_accum_too_large)
             << (isSigned ? 0 : 1);
       }
     }
 
-    uint64_t fract_part_as_int =
-        static_cast<uint64_t>(fract_part * (1ULL << fbits));
-    uint64_t final_fixed_point_as_int =
-        (int_part_as_int << fbits) + fract_part_as_int;
-
     llvm::APInt ResultVal(bit_width, final_fixed_point_as_int, isSigned);
     Res = FixedPointLiteral::CreateFromRawInt(Context, ResultVal, Ty,
                                               Tok.getLocation());
   } else if (Literal.isFloatingLiteral()) {
     QualType Ty;
     if (Literal.isHalf){
       if (getOpenCLOptions().isEnabled("cl_khr_fp16"))
         Ty = Context.HalfTy;

test/Frontend/fixed_point_all_builtin_operations.c

-// RUN: %clang_cc1 -Werror %s
+// RUN: %clang_cc1 -Werror -S -emit-llvm %s -o - | lli -force-interpreter=true
 
 // Check that we can use all supported binary and unary operations according to
 // clause 4.1.6 in N1169.
 
 #define ALL_OPERATIONS_FOR_TYPE(TYPE, ID)             \
   TYPE unary_add##ID(TYPE a) { return +a; }           \
   TYPE unary_sub##ID(TYPE a) { return -a; }           \
   int logical_not##ID(TYPE a) { return !a; }          \
#define ALL_OPERATIONS(TYPE, ID)                           \
   ALL_OPERATIONS_FOR_TYPE(_Sat unsigned TYPE, SatUnsigned##ID)
 
 ALL_OPERATIONS(short _Fract, ShortFract);
 ALL_OPERATIONS(_Fract, Fract);
 ALL_OPERATIONS(long _Fract, LongFract);
 ALL_OPERATIONS(short _Accum, ShortAccum);
 ALL_OPERATIONS(_Accum, Accum);
 ALL_OPERATIONS(long _Accum, LongAccum);
+
+#define ASSERT(x) \
+  if (!(x)) return 1;
+
+#define BINARY_OPS_FOR_TYPE(TYPE, ID, SUFFIX)         \
+  {                                                   \
+    TYPE a = 0.5##SUFFIX;                             \
+    TYPE b = 0.25##SUFFIX;                            \
+    ASSERT(add##ID(a, b) == 0.75##SUFFIX);            \
+    ASSERT(sub##ID(a, b) == 0.25##SUFFIX);            \
+    ASSERT(mul##ID(a, b) == 0.125##SUFFIX);           \
+    ASSERT(div##ID(b, a) == 0.5##SUFFIX);             \
+    ASSERT(shl##ID(b, 1) == a);                       \
+    ASSERT(shr##ID(a, 1) == b);                       \
+    ASSERT(lt##ID(b, a));                             \
+    ASSERT(le##ID(b, a));                             \
+    ASSERT(gt##ID(a, b));                             \
+    ASSERT(ge##ID(a, b));                             \
+    ASSERT(eq##ID(a, b) == 0);                        \
+    ASSERT(ne##ID(a, b));                             \
+    ASSERT(augmented_add##ID(a, b) == 0.75##SUFFIX);  \
+    ASSERT(augmented_sub##ID(a, b) == 0.25##SUFFIX);  \
+    ASSERT(augmented_mul##ID(a, b) == 0.125##SUFFIX); \
+    ASSERT(augmented_div##ID(b, a) == 0.5##SUFFIX);   \
+    ASSERT(augmented_shl##ID(b, 1) == a);             \
+    ASSERT(augmented_shr##ID(a, 1) == b);             \
+  }
+
+#define BINARY_OPS(TYPE, ID, SUFFIX)                            \
+  BINARY_OPS_FOR_TYPE(TYPE, ID, SUFFIX);                        \
+  BINARY_OPS_FOR_TYPE(signed TYPE, Signed##ID, SUFFIX);         \
+  BINARY_OPS_FOR_TYPE(unsigned TYPE, Unsigned##ID, u##SUFFIX);  \
+  BINARY_OPS_FOR_TYPE(_Sat TYPE, Sat##ID, SUFFIX);              \
+  BINARY_OPS_FOR_TYPE(_Sat signed TYPE, SatSigned##ID, SUFFIX); \
+  BINARY_OPS_FOR_TYPE(_Sat unsigned TYPE, SatUnsigned##ID, u##SUFFIX);
+
+#define FRACT_SAT_BINARY_OPS(TYPE, ID, SUFFIX) \
+  {                                            \
+    TYPE a = 0.7##SUFFIX;                      \
+    TYPE b = 0.9##SUFFIX;                      \
+    ASSERT(add##ID(a, b) == 1.0##SUFFIX);      \
+  }
+
+int main() {
+  BINARY_OPS(short _Fract, ShortFract, hr);
+  BINARY_OPS(_Fract, Fract, r);
+  BINARY_OPS(long _Fract, LongFract, lr);
+  BINARY_OPS(short _Accum, ShortAccum, hk);
+  BINARY_OPS(_Accum, Accum, k);
+  BINARY_OPS(long _Accum, LongAccum, lk);
+
+  FRACT_SAT_BINARY_OPS(_Sat short _Fract, SatShortFract, hr);
+  FRACT_SAT_BINARY_OPS(_Sat _Fract, SatFract, r);
+  FRACT_SAT_BINARY_OPS(_Sat long _Fract, SatLongFract, lr);
+
+  return 0;
+}

test/Frontend/fixed_point_builtin_macros.c

-// RUN: %clang_cc1 -S -emit-llvm -o - %s | lli
+// RUN: %clang_cc1 -S -emit-llvm -o - %s | lli -force-interpreter=true
 
 #define assert(b) if (!(b)) { return 1; }
 
 int main() {
   // Test using the recommended values for a typical desktop processor (Annex
   // A.3). These are also the default values when building clang.
   // Fractional bits of _Accum types
   assert(__SACCUM_FBIT__  == 7);