[Fixed Point Arithmetic] Fixed Point Addition Constant Expression Evaluation

[leonardchan]

Summary

This patch includes logic for constant expression evaluation of fixed point additions.

[rjmccall]

clang/include/clang/Basic/FixedPoint.h

98 ↗

(On Diff #178814)

This should be documented to describe what it does. Does it create a value with impossible semantics? Some reasonable default value in some reasonable default semantics?

clang/lib/AST/ExprConstant.cpp

9953 ↗

(On Diff #178814)

Can a fixed-point conversion ever have undefined behavior? If so, you might need to flag that as a failed case, unless we want to give it defined semantics in Clang.

9955 ↗

(On Diff #178814)

Do you not have a separate CK_IntegralToFixedPoint? It's convenient here, but still, it's curious.

You also need a CK_FloatingToFixedPoint, I think. (Obviously you don't need to implement that right now.)

9959 ↗

(On Diff #178814)

This is obviously unreachable because of the default case in the switch. Should this be moved into the default case, and then you can put your Error call in cases for the known-missing fixed-point conversions?

You should go ahead and add cases for CK_NoOp and CK_LValueToRValue, they should be obvious from the other evaluators.

You should add tests for constant-evaluating fixed-point conversions.

9982 ↗

(On Diff #178814)

This unreachable seems more reasonable since you're probably never going to make this a covered switch.

[ebevhan]

clang/include/clang/Basic/FixedPoint.h

98 ↗

(On Diff #178814)

As it is, it doesn't seem to create anything useful; neither does the one for the FixedPointSemantics. It would be a zero-width, zero-scale value.

clang/lib/AST/ExprConstant.cpp

1618 ↗

(On Diff #178814)

I think I would like to see both EvaluateFixedPoint and EvaluateFixedPointOrInteger, and use the former when you know it's supposed to be a fixed-point value (like in FixedPointCast).

9955 ↗

(On Diff #178814)

Both of those should exist. I think that the FixedPointCast evaluation should only use EvaluateFixedPoint.

9977 ↗

(On Diff #178814)

I understand the benefit of placing the actual operation implementation in the APFixedPoint class, but it means that any intermediate information is sort of lost. For example, if we want to warn the user about overflow (into the padding bit, or just in general) we can't, because that information was hidden.

I guess it could be done by checking if the result of the add is equal to the result of the convert for non-saturating ResultFXSema? The APFixedPoint comparison is value-based. Not sure how it would work with the padding bit in unsigned types, though.

clang/lib/Basic/FixedPoint.cpp

42 ↗

(On Diff #178814)

Maybe add a comment here to clarify what we are catching.

clang/test/Frontend/fixed_point_add.c

5 ↗

(On Diff #178814)

The test doesn't have a triple so the alignment might be affected by what machine the test runs on.

Now that I think about it, so will the results of the calculations, and the emitted IR in the functions.

[leonardchan]

I ended up adding a lot to this patch, so feel free to let me know if I should split this up if it's more difficult to review.

Changes:

• Remove default ctors for APFixedPoint and FixedPointSemantics. The main reason I had these was so in the EvaluateAsFixedPoint* functions, I could pass in an APFixedPoint by reference that could be assigned on successful evaluation. But it feels more correct now to instead just use an APValue.
• Allow APFixedPoint to be contained in APValue. This takes up a bulk of the new changes and could be it's own patch.
• FixedPointExprEvaluator now calculates an APValue that holds an APFixedPoint.
• Removed unused Success methods in FixedPointExprEvaluator.
• Added EvaluateAsFixedPoint functions and method to Expr.
• Added tests for constant-evaluating fixed point conversions.
• Added warning for potential overflow conversions when assigning to fixed point types (as a part of adding tests for conversions)
• APFixedPoint convert() and add() methods take an optional second argument for indicating if the operation resulted in an overflow (although this could be changed for a clearer way to check for operator overflow)

clang/include/clang/Basic/FixedPoint.h

98 ↗

(On Diff #178814)

The idea behind this was to enable a value to later be reassigned to an APFixedPoint, similar to how APValues are assigned in EvaluateInteger on success. I suppose the correct thing to do in this case would be to instead allow for APValue to take an APFixedPoint instead and pass that to EvaluateFixedPoint* to make it consistent with the other Evaluate methods and allow for APFixedPoint to be initialized with a default value.

clang/lib/AST/ExprConstant.cpp

9953 ↗

(On Diff #178814)

Yup. Overflow can happen for some non-saturating operations. Added.

9955 ↗

(On Diff #178814)

I have not yet added either of those but will do so in other patches.

9959 ↗

(On Diff #178814)

I think it would be better to keep it as an error since elsewhere, we emit errors indicating a particular operation or conversion has not yet been implemented.

9977 ↗

(On Diff #178814)

We could do something like having a separate method that checks for overflow or like bool addCanOverflow(APFixedPoint LHS, APFixedPoint RHS), or an additional optional pointer argument to the add method like APFixedPoint add(const APFixedPoint &Other, bool *overflow=nullptr) where if overflow is not a nullptr, we would still perform the addition but set this to indicate if we overflowed or not.

For now I went with the second option since this is useful now for checking overflow on casts and addition, but this could also be discussed/changed later.

9982 ↗

(On Diff #178814)

Should I just remove these? I recall seeing some other switch statements in clang where after all cases are covered, there's an llvm_unreachable and I only put this here for consistency.

clang/lib/Basic/FixedPoint.cpp

42 ↗

(On Diff #178814)

Added. I didn't know this before, but it turns out that isNegative() only checks if the last bit is set, but not if the value is signed or not.

clang/test/Frontend/fixed_point_add.c

5 ↗

(On Diff #178814)

Added

[ebevhan]

clang/lib/AST/ExprConstant.cpp

7568 ↗

(On Diff #181421)

This could provide an APFixedPoint rather than APValue.

7582 ↗

(On Diff #181421)

Technically, this will always produce an APFixedPoint so it doesn't really need to return APValue either.

7584 ↗

(On Diff #181421)

The semantic is not used in the fixed-point path.

9927 ↗

(On Diff #181421)

I think I've mentioned this before, but just as a reminder; this doesn't perform correctly for saturation.

9979 ↗

(On Diff #181421)

Perhaps this should call HandleOverflow (or do something else) to signal that overflow occurred. HandleOverflow only seems to emit a note, though, but I think it should probably be a warning.

Maybe for casts as well? Might get double warnings then, though.

clang/lib/Basic/FixedPoint.cpp

25 ↗

(On Diff #181421)

I think it's a bit cleaner if you avoid this variable and simply assign Overflow directly here, with the 'else' cases below replaced with else if (Overflow).

That style isn't cleaner in add if you use the APInt add_ov functions though, so maybe it's better to keep it this way for consistency.

170 ↗

(On Diff #181421)

You could use the add_ov functions here.

[rjmccall]

Yeah, I would recommend splitting the APFixedPoint in APValue changes into a separate patch.

clang/lib/AST/ExprConstant.cpp

9959 ↗

(On Diff #178814)

Okay.

[leonardchan]

In D55868#1358208, @rjmccall wrote:

Yeah, I would recommend splitting the APFixedPoint in APValue changes into a separate patch.

Added D56746 as a child revision of this that has the APValue changes.

clang/lib/AST/ExprConstant.cpp

9927 ↗

(On Diff #181421)

Added a negate() method to APFixedPoint which handles this now and accounts for saturation.

9979 ↗

(On Diff #181421)

Done. Double warnings don't seem to appear though.

clang/lib/Basic/FixedPoint.cpp

25 ↗

(On Diff #181421)

I'm not really bothered much by either style, but if we want to be consistent with the APInt overflow operations, we could just make the overflow parameter a reference also while it's still new/under work.

[rjmccall]

LGTM.

include/clang/AST/Expr.h

 #include "clang/AST/ASTVector.h"
 #include "clang/AST/Decl.h"
 #include "clang/AST/DeclAccessPair.h"
 #include "clang/AST/OperationKinds.h"
 #include "clang/AST/Stmt.h"
 #include "clang/AST/TemplateBase.h"
 #include "clang/AST/Type.h"
 #include "clang/Basic/CharInfo.h"
+#include "clang/Basic/FixedPoint.h"
 #include "clang/Basic/LangOptions.h"
 #include "clang/Basic/SyncScope.h"
 #include "clang/Basic/TypeTraits.h"
 #include "llvm/ADT/APFloat.h"
 #include "llvm/ADT/APSInt.h"
 #include "llvm/ADT/SmallVector.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Support/AtomicOrdering.h"
public:
 
   /// EvaluateAsFloat - Return true if this is a constant which we can fold and
   /// convert to a floating point value, using any crazy technique that we
   /// want to.
   bool
   EvaluateAsFloat(llvm::APFloat &Result, const ASTContext &Ctx,
                   SideEffectsKind AllowSideEffects = SE_NoSideEffects) const;
 
+  /// EvaluateAsFloat - Return true if this is a constant which we can fold and
+  /// convert to a fixed point value.
+  bool EvaluateAsFixedPoint(
+      EvalResult &Result, const ASTContext &Ctx,
+      SideEffectsKind AllowSideEffects = SE_NoSideEffects) const;
+
   /// isEvaluatable - Call EvaluateAsRValue to see if this expression can be
   /// constant folded without side-effects, but discard the result.
   bool isEvaluatable(const ASTContext &Ctx,
                      SideEffectsKind AllowSideEffects = SE_NoSideEffects) const;
 
   /// HasSideEffects - This routine returns true for all those expressions
   /// which have any effect other than producing a value. Example is a function
   /// call, volatile variable read, or throwing an exception. If

include/clang/Basic/DiagnosticGroups.td

 def PointerBoolConversion : DiagGroup<"pointer-bool-conversion">;
 def UndefinedBoolConversion : DiagGroup<"undefined-bool-conversion">;
 def BoolConversion : DiagGroup<"bool-conversion", [PointerBoolConversion,
                                                    UndefinedBoolConversion]>;
 def IntConversion : DiagGroup<"int-conversion">;
 def EnumConversion : DiagGroup<"enum-conversion">;
 def ImplicitIntConversion : DiagGroup<"implicit-int-conversion">;
 def ImplicitFloatConversion : DiagGroup<"implicit-float-conversion">;
+def ImplicitFixedPointConversion : DiagGroup<"implicit-fixed-point-conversion">;
 
 def FloatOverflowConversion : DiagGroup<"float-overflow-conversion">;
 def FloatZeroConversion : DiagGroup<"float-zero-conversion">;
 def FloatConversion :
   DiagGroup<"float-conversion", [FloatOverflowConversion,
                                  FloatZeroConversion]>;
 
 def DoublePromotion : DiagGroup<"double-promotion">;

include/clang/Basic/DiagnosticSemaKinds.td

def warn_impcast_integer_64_32 : Warning<
   InGroup<Shorten64To32>, DefaultIgnore;
 def warn_impcast_integer_precision_constant : Warning<
   "implicit conversion from %2 to %3 changes value from %0 to %1">,
   InGroup<ConstantConversion>;
 def warn_impcast_bitfield_precision_constant : Warning<
   "implicit truncation from %2 to bit-field changes value from %0 to %1">,
   InGroup<BitFieldConstantConversion>;
 
+def warn_impcast_fixed_point_range : Warning<
+  "implicit conversion from %0 cannot fit within the range of values for %1">,
+  InGroup<ImplicitFixedPointConversion>;
+
 def warn_impcast_literal_float_to_integer : Warning<
   "implicit conversion from %0 to %1 changes value from %2 to %3">,
   InGroup<LiteralConversion>;
 def warn_impcast_literal_float_to_integer_out_of_range : Warning<
   "implicit conversion of out of range value from %0 to %1 is undefined">,
   InGroup<LiteralConversion>;
 def warn_impcast_float_integer : Warning<
   "implicit conversion turns floating-point number into integer: %0 to %1">,

include/clang/Basic/FixedPoint.h

public:
    inline unsigned getScale() const { return Sema.getScale(); }
    inline bool isSaturated() const { return Sema.isSaturated(); }
    inline bool isSigned() const { return Sema.isSigned(); }
    inline bool hasPadding() const { return Sema.hasUnsignedPadding(); }
    FixedPointSemantics getSemantics() const { return Sema; }
 
    bool getBoolValue() const { return Val.getBoolValue(); }
 
-   // Convert this number to match the semantics provided.
-   APFixedPoint convert(const FixedPointSemantics &DstSema) const;
+   // Convert this number to match the semantics provided. If the overflow
+   // parameter is provided, set this value to true or false to indicate if this
+   // operation results in an overflow.
+   APFixedPoint convert(const FixedPointSemantics &DstSema,
+                        bool *Overflow = nullptr) const;
+
+   // Perform binary operations on a fixed point type. The resulting fixed point
+   // value will be in the common, full precision semantics that can represent
+   // the precision and ranges os both input values. See convert() for an
+   // explanation of the Overflow parameter.
+   APFixedPoint add(const APFixedPoint &Other, bool *Overflow = nullptr) const;
+
+   /// Perform a unary negation (-X) on this fixed point type, taking into
+   /// account saturation if applicable.
+   APFixedPoint negate(bool *Overflow = nullptr) const;
 
    APFixedPoint shr(unsigned Amt) const {
      return APFixedPoint(Val >> Amt, Sema);
    }
 
   APFixedPoint shl(unsigned Amt) const {
     return APFixedPoint(Val << Amt, Sema);
   }

lib/AST/ExprConstant.cpp

 #include "clang/AST/ASTLambda.h"
 #include "clang/AST/CharUnits.h"
 #include "clang/AST/Expr.h"
 #include "clang/AST/OSLog.h"
 #include "clang/AST/RecordLayout.h"
 #include "clang/AST/StmtVisitor.h"
 #include "clang/AST/TypeLoc.h"
 #include "clang/Basic/Builtins.h"
+#include "clang/Basic/FixedPoint.h"
 #include "clang/Basic/TargetInfo.h"
 #include "llvm/Support/SaveAndRestore.h"
 #include "llvm/Support/raw_ostream.h"
 #include <cstring>
 #include <functional>
 
 #define DEBUG_TYPE "exprconstant"
 
OptionalDiagnostic &operator<<(const APFloat &F) {
             llvm::APFloat::semanticsPrecision(F.getSemantics());
         precision = (precision * 59 + 195) / 196;
         SmallVector<char, 32> Buffer;
         F.toString(Buffer, precision);
         *Diag << StringRef(Buffer.data(), Buffer.size());
       }
       return *this;
     }
+
+    OptionalDiagnostic &operator<<(const APFixedPoint &FX) {
+      if (Diag) {
+        SmallVector<char, 32> Buffer;
+        FX.toString(Buffer);
+        *Diag << StringRef(Buffer.data(), Buffer.size());
+      }
+      return *this;
+    }
   };
 
   /// A cleanup, and a flag indicating whether it is lifetime-extended.
   class Cleanup {
     llvm::PointerIntPair<APValue*, 1, bool> Value;
 
   public:
     Cleanup(APValue *Val, bool IsLifetimeExtended)

 static bool EvaluateIntegerOrLValue(const Expr *E, APValue &Result,
                                     EvalInfo &Info);
 static bool EvaluateFloat(const Expr *E, APFloat &Result, EvalInfo &Info);
 static bool EvaluateComplex(const Expr *E, ComplexValue &Res, EvalInfo &Info);
 static bool EvaluateAtomic(const Expr *E, const LValue *This, APValue &Result,
                            EvalInfo &Info);
 static bool EvaluateAsRValue(EvalInfo &Info, const Expr *E, APValue &Result);
 
+/// Evaluate an integer or fixed point expression into an APResult.
+static bool EvaluateFixedPointOrInteger(const Expr *E, APFixedPoint &Result,
+                                        EvalInfo &Info);
+
+/// Evaluate only a fixed point expression into an APResult.
+static bool EvaluateFixedPoint(const Expr *E, APFixedPoint &Result,
+                               EvalInfo &Info);
+
 //===----------------------------------------------------------------------===//
 // Misc utilities
 //===----------------------------------------------------------------------===//
 
 /// A helper function to create a temporary and set an LValue.
 template <class KeyTy>
 static APValue &createTemporary(const KeyTy *Key, bool IsLifetimeExtended,
                                 LValue &LV, CallStackFrame &Frame) {

 class FixedPointExprEvaluator
     : public ExprEvaluatorBase<FixedPointExprEvaluator> {
   APValue &Result;
 
  public:
   FixedPointExprEvaluator(EvalInfo &info, APValue &result)
       : ExprEvaluatorBaseTy(info), Result(result) {}
 
-  bool Success(const llvm::APSInt &SI, const Expr *E, APValue &Result) {
-    assert(E->getType()->isFixedPointType() && "Invalid evaluation result.");
-    assert(SI.isSigned() == E->getType()->isSignedFixedPointType() &&
-           "Invalid evaluation result.");
-    assert(SI.getBitWidth() == Info.Ctx.getIntWidth(E->getType()) &&
-           "Invalid evaluation result.");
-    Result = APValue(SI);
-    return true;
-  }
-  bool Success(const llvm::APSInt &SI, const Expr *E) {
-    return Success(SI, E, Result);
-  }
-
-  bool Success(const llvm::APInt &I, const Expr *E, APValue &Result) {
-    assert(E->getType()->isFixedPointType() && "Invalid evaluation result.");
-    assert(I.getBitWidth() == Info.Ctx.getIntWidth(E->getType()) &&
-           "Invalid evaluation result.");
-    Result = APValue(APSInt(I));
-    Result.getInt().setIsUnsigned(E->getType()->isUnsignedFixedPointType());
-    return true;
-  }
   bool Success(const llvm::APInt &I, const Expr *E) {
-    return Success(I, E, Result);
+    return Success(
+        APFixedPoint(I, Info.Ctx.getFixedPointSemantics(E->getType())), E);
   }
 
-  bool Success(uint64_t Value, const Expr *E, APValue &Result) {
-    assert(E->getType()->isFixedPointType() && "Invalid evaluation result.");
-    Result = APValue(Info.Ctx.MakeIntValue(Value, E->getType()));
-    return true;
-  }
   bool Success(uint64_t Value, const Expr *E) {
-    return Success(Value, E, Result);
+    return Success(
+        APFixedPoint(Value, Info.Ctx.getFixedPointSemantics(E->getType())), E);
   }
 
-  bool Success(CharUnits Size, const Expr *E) {
-    return Success(Size.getQuantity(), E);
+  bool Success(const APValue &V, const Expr *E) {
+    return Success(V.getFixedPoint(), E);
   }
 
-  bool Success(const APValue &V, const Expr *E) {
-    if (V.isLValue() || V.isAddrLabelDiff()) {
-      Result = V;
-      return true;
-    }
-    return Success(V.getInt(), E);
-  }
-
-  bool ZeroInitialization(const Expr *E) { return Success(0, E); }
+  bool Success(const APFixedPoint &V, const Expr *E) {
+    assert(E->getType()->isFixedPointType() && "Invalid evaluation result.");
+    assert(V.getWidth() == Info.Ctx.getIntWidth(E->getType()) &&
+           "Invalid evaluation result.");
+    Result = APValue(V);
 
   //===--------------------------------------------------------------------===//
   //                            Visitor Methods
   //===--------------------------------------------------------------------===//
 
   bool VisitFixedPointLiteral(const FixedPointLiteral *E) {
     return Success(E->getValue(), E);
   }
 
+  bool VisitCastExpr(const CastExpr *E);
   bool VisitUnaryOperator(const UnaryOperator *E);
+  bool VisitBinaryOperator(const BinaryOperator *E);
 };
 } // end anonymous namespace
 
 /// EvaluateIntegerOrLValue - Evaluate an rvalue integral-typed expression, and
 /// produce either the integer value or a pointer.
 ///
 /// GCC has a heinous extension which folds casts between pointer types and
 /// pointer-sized integral types. We support this by allowing the evaluation of
if (!Val.isInt()) {
     //        a pointer to an integer.
     Info.FFDiag(E, diag::note_invalid_subexpr_in_const_expr);
     return false;
   }
   Result = Val.getInt();
   return true;
 }
 
+static bool EvaluateFixedPoint(const Expr *E, APFixedPoint &Result,
+                               EvalInfo &Info) {
+  if (E->getType()->isFixedPointType()) {
+    APValue Val;
+    if (!FixedPointExprEvaluator(Info, Val).Visit(E))
+      return false;
+    if (!Val.isFixedPoint())
+      return false;
+
+    Result = Val.getFixedPoint();
+    return true;
+  }
+  return false;
+}
+
+static bool EvaluateFixedPointOrInteger(const Expr *E, APFixedPoint &Result,
+                                        EvalInfo &Info) {
+  if (E->getType()->isIntegerType()) {
+    auto FXSema = Info.Ctx.getFixedPointSemantics(E->getType());
+    APSInt Val;
+    if (!EvaluateInteger(E, Val, Info))
+      return false;
+    Result = APFixedPoint(Val, FXSema);
+    return true;
+  } else if (E->getType()->isFixedPointType()) {
+    return EvaluateFixedPoint(E, Result, Info);
+  }
+  return false;
+}
+
 /// Check whether the given declaration can be directly converted to an integral
 /// rvalue. If not, no diagnostic is produced; there are other things we can
 /// try.
 bool IntExprEvaluator::CheckReferencedDecl(const Expr* E, const Decl* D) {
   // Enums are integer constant exprs.
   if (const EnumConstantDecl *ECD = dyn_cast<EnumConstantDecl>(D)) {
     // Check for signedness/width mismatches between E type and ECD value.
     bool SameSign = (ECD->getInitVal().isSigned()
case CK_IntegralComplexToBoolean: {
     return Success(IntResult, E);
   }
 
   case CK_FixedPointToBoolean: {
     // Unsigned padding does not affect this.
     APValue Val;
     if (!Evaluate(Val, Info, SubExpr))
       return false;
-    return Success(Val.getInt().getBoolValue(), E);
+    return Success(Val.getFixedPoint().getBoolValue(), E);
   }
 
   case CK_IntegralCast: {
     if (!Visit(SubExpr))
       return false;
 
     if (!Result.isInt()) {
       // Allow casts of address-of-label differences if they are no-ops
switch (E->getOpcode()) {
     default:
       // Invalid unary operators
       return Error(E);
     case UO_Plus:
       // The result is just the value.
       return Visit(E->getSubExpr());
     case UO_Minus: {
       if (!Visit(E->getSubExpr())) return false;
-      if (!Result.isInt()) return Error(E);
-      const APSInt &Value = Result.getInt();
-      if (Value.isSigned() && Value.isMinSignedValue() && E->canOverflow()) {
-        SmallString<64> S;
-        FixedPointValueToString(S, Value,
-                                Info.Ctx.getTypeInfo(E->getType()).Width);
-        Info.CCEDiag(E, diag::note_constexpr_overflow) << S << E->getType();
-        if (Info.noteUndefinedBehavior()) return false;
-      }
-      return Success(-Value, E);
+      if (!Result.isFixedPoint())
+        return Error(E);
+      bool Overflowed;
+      APFixedPoint Negated = Result.getFixedPoint().negate(&Overflowed);
+      if (Overflowed && !HandleOverflow(Info, E, Negated, E->getType()))
+        return false;
+      return Success(Negated, E);
     }
     case UO_LNot: {
       bool bres;
       if (!EvaluateAsBooleanCondition(E->getSubExpr(), bres, Info))
         return false;
       return Success(!bres, E);
     }
   }
 }
 
+bool FixedPointExprEvaluator::VisitCastExpr(const CastExpr *E) {
+  const Expr *SubExpr = E->getSubExpr();
+  QualType DestType = E->getType();
+  assert(DestType->isFixedPointType() &&
+         "Expected destination type to be a fixed point type");
+  auto DestFXSema = Info.Ctx.getFixedPointSemantics(DestType);
+
+  switch (E->getCastKind()) {
+  case CK_FixedPointCast: {
+    APFixedPoint Src(Info.Ctx.getFixedPointSemantics(SubExpr->getType()));
+    if (!EvaluateFixedPoint(SubExpr, Src, Info))
+      return false;
+    bool Overflowed;
+    APFixedPoint Result = Src.convert(DestFXSema, &Overflowed);
+    if (Overflowed && !HandleOverflow(Info, E, Result, DestType))
+      return false;
+    return Success(Result, E);
+  }
+  case CK_NoOp:
+  case CK_LValueToRValue:
+    return ExprEvaluatorBaseTy::VisitCastExpr(E);
+  default:
+    return Error(E);
+  }
+}
+
+bool FixedPointExprEvaluator::VisitBinaryOperator(const BinaryOperator *E) {
+  const Expr *LHS = E->getLHS();
+  const Expr *RHS = E->getRHS();
+  FixedPointSemantics ResultFXSema =
+      Info.Ctx.getFixedPointSemantics(E->getType());
+
+  APFixedPoint LHSFX(Info.Ctx.getFixedPointSemantics(LHS->getType()));
+  if (!EvaluateFixedPointOrInteger(LHS, LHSFX, Info))
+    return false;
+  APFixedPoint RHSFX(Info.Ctx.getFixedPointSemantics(RHS->getType()));
+  if (!EvaluateFixedPointOrInteger(RHS, RHSFX, Info))
+    return false;
+
+  switch (E->getOpcode()) {
+  case BO_Add: {
+    bool AddOverflow, ConversionOverflow;
+    APFixedPoint Result = LHSFX.add(RHSFX, &AddOverflow)
+                              .convert(ResultFXSema, &ConversionOverflow);
+    if ((AddOverflow || ConversionOverflow) &&
+        !HandleOverflow(Info, E, Result, E->getType()))
+      return false;
+    return Success(Result, E);
+  }
+  default:
+    return false;
+  }
+  llvm_unreachable("Should've exited before this");
+}
+
 //===----------------------------------------------------------------------===//
 // Float Evaluation
 //===----------------------------------------------------------------------===//
 
 namespace {
 class FloatExprEvaluator
   : public ExprEvaluatorBase<FloatExprEvaluator> {
   APFloat &Result;
static bool EvaluateAsInt(const Expr *E, Expr::EvalResult &ExprResult,
   if (!::EvaluateAsRValue(E, ExprResult, Ctx, Info) ||
       !ExprResult.Val.isInt() ||
       hasUnacceptableSideEffect(ExprResult, AllowSideEffects))
     return false;
 
   return true;
 }
 
+static bool EvaluateAsFixedPoint(const Expr *E, Expr::EvalResult &ExprResult,
+                                 const ASTContext &Ctx,
+                                 Expr::SideEffectsKind AllowSideEffects,
+                                 EvalInfo &Info) {
+  if (!E->getType()->isFixedPointType())
+    return false;
+
+  if (!::EvaluateAsRValue(E, ExprResult, Ctx, Info))
+    return false;
+
+  if (!ExprResult.Val.isFixedPoint() ||
+      hasUnacceptableSideEffect(ExprResult, AllowSideEffects))
+    return false;
+
+  return true;
+}
+
 /// EvaluateAsRValue - Return true if this is a constant which we can fold using
 /// any crazy technique (that has nothing to do with language standards) that
 /// we want to.  If this function returns true, it returns the folded constant
 /// in Result. If this expression is a glvalue, an lvalue-to-rvalue conversion
 /// will be applied to the result.
 bool Expr::EvaluateAsRValue(EvalResult &Result, const ASTContext &Ctx,
                             bool InConstantContext) const {
   EvalInfo Info(Ctx, Result, EvalInfo::EM_IgnoreSideEffects);

 }
 
 bool Expr::EvaluateAsInt(EvalResult &Result, const ASTContext &Ctx,
                          SideEffectsKind AllowSideEffects) const {
   EvalInfo Info(Ctx, Result, EvalInfo::EM_IgnoreSideEffects);
   return ::EvaluateAsInt(this, Result, Ctx, AllowSideEffects, Info);
 }
 
+bool Expr::EvaluateAsFixedPoint(EvalResult &Result, const ASTContext &Ctx,
+                                SideEffectsKind AllowSideEffects) const {
+  EvalInfo Info(Ctx, Result, EvalInfo::EM_IgnoreSideEffects);
+  return ::EvaluateAsFixedPoint(this, Result, Ctx, AllowSideEffects, Info);
+}
+
 bool Expr::EvaluateAsFloat(APFloat &Result, const ASTContext &Ctx,
                            SideEffectsKind AllowSideEffects) const {
   if (!getType()->isRealFloatingType())
     return false;
 
   EvalResult ExprResult;
   if (!EvaluateAsRValue(ExprResult, Ctx) || !ExprResult.Val.isFloat() ||
       hasUnacceptableSideEffect(ExprResult, AllowSideEffects))

lib/Basic/FixedPoint.cpp

 /// Defines the implementation for the fixed point number interface.
 //
 //===----------------------------------------------------------------------===//
 
 #include "clang/Basic/FixedPoint.h"
 
 namespace clang {
 
-APFixedPoint APFixedPoint::convert(const FixedPointSemantics &DstSema) const {
+APFixedPoint APFixedPoint::convert(const FixedPointSemantics &DstSema,
+                                   bool *Overflow) const {
   llvm::APSInt NewVal = Val;
   unsigned DstWidth = DstSema.getWidth();
   unsigned DstScale = DstSema.getScale();
   bool Upscaling = DstScale > getScale();
+  if (Overflow)
+    *Overflow = false;
 
   if (Upscaling) {
     NewVal = NewVal.extend(NewVal.getBitWidth() + DstScale - getScale());
     NewVal <<= (DstScale - getScale());
   } else {
     NewVal >>= (getScale() - DstScale);
   }
 
-  if (DstSema.isSaturated()) {
-    auto Mask = llvm::APInt::getBitsSetFrom(
-        NewVal.getBitWidth(),
-        std::min(DstScale + DstSema.getIntegralBits(), NewVal.getBitWidth()));
-    llvm::APInt Masked(NewVal & Mask);
 
-    // Change in the bits above the sign
-    if (!(Masked == Mask || Masked == 0))
+  if (!(Masked == Mask || Masked == 0)) {
+    // Found overflow in the bits above the sign
+    if (DstSema.isSaturated())
       NewVal = NewVal.isNegative() ? Mask : ~Mask;
+    else if (Overflow)
+      *Overflow = true;
+  }
 
-    if (!DstSema.isSigned() && NewVal.isNegative())
+  // If the dst semantics are unsigned, but our value is signed and negative, we
+  // clamp to zero.
+  if (!DstSema.isSigned() && NewVal.isSigned() && NewVal.isNegative()) {
+    // Found negative overflow for unsigned result
+    if (DstSema.isSaturated())
       NewVal = 0;
+    else if (Overflow)
+      *Overflow = true;
   }
 
   NewVal = NewVal.extOrTrunc(DstWidth);
   NewVal.setIsSigned(DstSema.isSigned());
   return APFixedPoint(NewVal, DstSema);
 }
 
 int APFixedPoint::compare(const APFixedPoint &Other) const {
FixedPointSemantics FixedPointSemantics::getCommonSemantics(
   // bit back if we are not saturating.
   if (ResultIsSigned || ResultHasUnsignedPadding)
     CommonWidth++;
 
   return FixedPointSemantics(CommonWidth, CommonScale, ResultIsSigned,
                              ResultIsSaturated, ResultHasUnsignedPadding);
 }
 
+APFixedPoint APFixedPoint::add(const APFixedPoint &Other,
+                               bool *Overflow) const {
+  auto CommonFXSema = Sema.getCommonSemantics(Other.getSemantics());
+  APFixedPoint ConvertedThis = convert(CommonFXSema);
+  APFixedPoint ConvertedOther = Other.convert(CommonFXSema);
+  llvm::APSInt ThisVal = ConvertedThis.getValue();
+  llvm::APSInt OtherVal = ConvertedOther.getValue();
+  bool Overflowed = false;
+
+  llvm::APSInt Result;
+  if (CommonFXSema.isSaturated()) {
+    Result = CommonFXSema.isSigned() ? ThisVal.sadd_sat(OtherVal)
+                                     : ThisVal.uadd_sat(OtherVal);
+  } else {
+    Result = ThisVal.isSigned() ? ThisVal.sadd_ov(OtherVal, Overflowed)
+                                : ThisVal.uadd_ov(OtherVal, Overflowed);
+  }
+
+  if (Overflow)
+    *Overflow = Overflowed;
+
+  return APFixedPoint(Result, CommonFXSema);
+}
+
 void APFixedPoint::toString(llvm::SmallVectorImpl<char> &Str) const {
   llvm::APSInt Val = getValue();
   unsigned Scale = getScale();
 
   if (Val.isSigned() && Val.isNegative() && Val != -Val) {
     Val = -Val;
     Str.push_back('-');
   }
void APFixedPoint::toString(llvm::SmallVectorImpl<char> &Str) const {
   do {
     (FractPart * RadixInt)
         .lshr(Scale)
         .toString(Str, /*radix=*/10, Val.isSigned());
     FractPart = (FractPart * RadixInt) & FractPartMask;
   } while (FractPart != 0);
 }
 
+APFixedPoint APFixedPoint::negate(bool *Overflow) const {
+  if (!isSaturated()) {
+    if (Overflow)
+      *Overflow =
+          (!isSigned() && Val != 0) || (isSigned() && Val.isMinSignedValue());
+    return APFixedPoint(-Val, Sema);
+  }
+
+  // We never overflow for saturation
+  if (Overflow)
+    *Overflow = false;
+
+  if (isSigned())
+    return Val.isMinSignedValue() ? getMax(Sema) : APFixedPoint(-Val, Sema);
+  else
+    return APFixedPoint(Sema);
+}
+
 }  // namespace clang

lib/Sema/SemaChecking.cpp

if (Target->isBooleanType() && isa<CallExpr>(E)) {
           DiagnoseImpCast(S, E, T, CC,
                           diag::warn_impcast_floating_point_to_bool);
         }
       }
     }
     return;
   }
 
+  if (Source->isFixedPointType()) {
+    // TODO: Only CK_FixedPointCast is supported now. The other valid casts
+    // should be accounted for here.
+    if (Target->isFixedPointType()) {
+      Expr::EvalResult Result;
+      if (E->EvaluateAsFixedPoint(Result, S.Context,
+                                  Expr::SE_AllowSideEffects)) {
+        APFixedPoint Value = Result.Val.getFixedPoint();
+        APFixedPoint MaxVal = S.Context.getFixedPointMax(T);
+        APFixedPoint MinVal = S.Context.getFixedPointMin(T);
+        if (Value > MaxVal || Value < MinVal) {
+          S.DiagRuntimeBehavior(E->getExprLoc(), E,
+                                S.PDiag(diag::warn_impcast_fixed_point_range)
+                                    << Value.toString() << T
+                                    << E->getSourceRange()
+                                    << clang::SourceRange(CC));
+          return;
+        }
+      }
+    }
+  }
+
   DiagnoseNullConversion(S, E, T, CC);
 
   S.DiscardMisalignedMemberAddress(Target, E);
 
   if (!Source->isIntegerType() || !Target->isIntegerType())
     return;
 
   // TODO: remove this early return once the false positives for constant->bool

test/Frontend/fixed_point_add.c

-// RUN: %clang_cc1 -ffixed-point -S -emit-llvm %s -o - | FileCheck %s --check-prefixes=CHECK,SIGNED
-// RUN: %clang_cc1 -ffixed-point -fpadding-on-unsigned-fixed-point -S -emit-llvm %s -o - | FileCheck %s --check-prefixes=CHECK,UNSIGNED
+// RUN: %clang_cc1 -ffixed-point -triple x86_64-unknown-linux-gnu -S -emit-llvm %s -o - | FileCheck %s --check-prefixes=CHECK,SIGNED
+// 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,UNSIGNED
+
+// Addition between different fixed point types
+short _Accum sa_const = 1.0hk + 2.0hk;  // CHECK-DAG: @sa_const  = {{.*}}global i16 384, align 2
+_Accum a_const = 1.0hk + 2.0k;          // CHECK-DAG: @a_const   = {{.*}}global i32 98304, align 4
+long _Accum la_const = 1.0hk + 2.0lk;   // CHECK-DAG: @la_const  = {{.*}}global i64 6442450944, align 8
+short _Accum sa_const2 = 0.5hr + 2.0hk; // CHECK-DAG: @sa_const2  = {{.*}}global i16 320, align 2
+short _Accum sa_const3 = 0.5r + 2.0hk;  // CHECK-DAG: @sa_const3  = {{.*}}global i16 320, align 2
+short _Accum sa_const4 = 0.5lr + 2.0hk; // CHECK-DAG: @sa_const4  = {{.*}}global i16 320, align 2
+
+// Unsigned addition
+unsigned short _Accum usa_const = 1.0uhk + 2.0uhk;
+// CHECK-SIGNED-DAG:   @usa_const = {{.*}}global i16 768, align 2
+// CHECK-UNSIGNED-DAG: @usa_const = {{.*}}global i16 384, align 2
+
+// Unsigned + signed
+short _Accum sa_const5 = 1.0uhk + 2.0hk;
+// CHECK-DAG: @sa_const5 = {{.*}}global i16 384, align 2
+
+// Addition with negative number
+short _Accum sa_const6 = 0.5hr + (-2.0hk);
+// CHECK-DAG: @sa_const6 = {{.*}}global i16 -192, align 2
+
+// Int addition
+unsigned short _Accum usa_const2 = 2 + 0.5uhk;
+// CHECK-SIGNED-DAG:   @usa_const2 = {{.*}}global i16 640, align 2
+// CHECK-UNSIGNED-DAG: @usa_const2 = {{.*}}global i16 320, align 2
+short _Accum sa_const7 = 2 + (-0.5hk);   // CHECK-DAG: @sa_const7 = {{.*}}global i16 192, align 2
+short _Accum sa_const8 = 257 + (-2.0hk); // CHECK-DAG: @sa_const8 = {{.*}}global i16 32640, align 2
+long _Fract lf_const = -0.5lr + 1;       // CHECK-DAG: @lf_const  = {{.*}}global i32 1073741824, align 4
+
+// Saturated addition
+_Sat short _Accum sat_sa_const = (_Sat short _Accum)128.0hk + 128.0hk;
+// CHECK-DAG: @sat_sa_const = {{.*}}global i16 32767, align 2
+_Sat unsigned short _Accum sat_usa_const = (_Sat unsigned short _Accum)128.0uhk + 128.0uhk;
+// CHECK-SIGNED-DAG:   @sat_usa_const = {{.*}}global i16 65535, align 2
+// CHECK-UNSIGNED-DAG: @sat_usa_const = {{.*}}global i16 32767, align 2
+_Sat short _Accum sat_sa_const2 = (_Sat short _Accum)128.0hk + 128;
+// CHECK-DAG: @sat_sa_const2 = {{.*}}global i16 32767, align 2
+_Sat unsigned short _Accum sat_usa_const2 = (_Sat unsigned short _Accum)128.0uhk + 128;
+// CHECK-SIGNED-DAG:   @sat_usa_const2 = {{.*}}global i16 65535, align 2
+// CHECK-UNSIGNED-DAG: @sat_usa_const2 = {{.*}}global i16 32767, align 2
+_Sat unsigned short _Accum sat_usa_const3 = (_Sat unsigned short _Accum)0.5uhk + (-2);
+// CHECK-DAG:   @sat_usa_const3 = {{.*}}global i16 0, align 2
 
 void SignedAddition() {
   // CHECK-LABEL: SignedAddition
   short _Accum sa;
   _Accum a, b, c, d;
   long _Accum la;
   unsigned short _Accum usa;
   unsigned _Accum ua;

test/Frontend/fixed_point_conversions.c

-// RUN: %clang_cc1 -ffixed-point -S -emit-llvm %s -o - | FileCheck %s -check-prefix=DEFAULT
-// RUN: %clang_cc1 -ffixed-point -S -emit-llvm %s -o - -fpadding-on-unsigned-fixed-point | FileCheck %s -check-prefix=SAME
+// RUN: %clang_cc1 -ffixed-point -triple x86_64-unknown-linux-gnu -S -emit-llvm %s -o - | FileCheck %s -check-prefix=DEFAULT
+// RUN: %clang_cc1 -ffixed-point -triple x86_64-unknown-linux-gnu -S -emit-llvm %s -o - -fpadding-on-unsigned-fixed-point | FileCheck %s -check-prefix=SAME
+
+// Between different fixed point types
+short _Accum sa_const = 2.5hk; // DEFAULT-DAG: @sa_const  = {{.*}}global i16 320, align 2
+_Accum a_const = 2.5hk;        // DEFAULT-DAG: @a_const   = {{.*}}global i32 81920, align 4
+short _Accum sa_const2 = 2.5k; // DEFAULT-DAG: @sa_const2 = {{.*}}global i16 320, align 2
+
+short _Accum sa_from_f_const = 0.5r; // DEFAULT-DAG: sa_from_f_const = {{.*}}global i16 64, align 2
+_Fract f_from_sa_const = 0.5hk;      // DEFAULT-DAG: f_from_sa_const = {{.*}}global i16 16384, align 2
+
+unsigned short _Accum usa_const = 2.5uk;
+unsigned _Accum ua_const = 2.5uhk;
+// DEFAULT-DAG: @usa_const  = {{.*}}global i16 640, align 2
+// DEFAULT-DAG: @ua_const   = {{.*}}global i32 163840, align 4
+// SAME-DAG:    @usa_const  = {{.*}}global i16 320, align 2
+// SAME-DAG:    @ua_const   = {{.*}}global i32 81920, align 4
+
+// Signedness
+unsigned short _Accum usa_const2 = 2.5hk;
+// DEFAULT-DAG: @usa_const2  = {{.*}}global i16 640, align 2
+// SAME-DAG:    @usa_const2  = {{.*}}global i16 320, align 2
+short _Accum sa_const3 = 2.5hk; // DEFAULT-DAG: @sa_const3 = {{.*}}global i16 320, align 2
+
+// Overflow (this is undefined but allowed)
+short _Accum sa_const4 = 256.0k;
+
+// Saturation
+_Sat short _Accum sat_sa_const = 2.5hk;   // DEFAULT-DAG: @sat_sa_const  = {{.*}}global i16 320, align 2
+_Sat short _Accum sat_sa_const2 = 256.0k; // DEFAULT-DAG: @sat_sa_const2 = {{.*}}global i16 32767, align 2
+_Sat unsigned short _Accum sat_usa_const = -1.0hk;
+// DEFAULT-DAG: @sat_usa_const = {{.*}}global i16 0, align 2
+// SAME-DAG:    @sat_usa_const = {{.*}}global i16 0, align 2
+_Sat unsigned short _Accum sat_usa_const2 = 256.0k;
+// DEFAULT-DAG: @sat_usa_const2 = {{.*}}global i16 -1, align 2
+// SAME-DAG:    @sat_usa_const2 = {{.*}}global i16 32767, align 2
 
 void TestFixedPointCastSameType() {
   _Accum a = 2.5k;
   _Accum a2 = a;
   // DEFAULT:      [[ACCUM:%[0-9a-z]+]] = load i32, i32* %a, align 4
   // DEFAULT-NEXT: store i32 [[ACCUM]], i32* %a2, align 4
 
   a2 = (_Accum)a;

test/Frontend/fixed_point_errors.c

void CheckSuffixOnIntegerLiterals() {
   u_long_fract_int = 0x10ulr;  // expected-error{{invalid suffix 'ulr' on integer constant}}
 
   // Using auto
   auto auto_fract = 0r;  // expected-error{{invalid suffix 'r' on integer constant}}
                          // expected-warning@-1{{type specifier missing, defaults to 'int'}}
   auto auto_accum = 0k;  // expected-error{{invalid suffix 'k' on integer constant}}
                          // expected-warning@-1{{type specifier missing, defaults to 'int'}}
 }
+
+// Overflow
+short _Accum sa_const = 256.0k;   // expected-warning{{implicit conversion from 256.0 cannot fit within the range of values for 'short _Accum'}}
+short _Fract sf_const = 1.0hk;    // expected-warning{{implicit conversion from 1.0 cannot fit within the range of values for 'short _Fract'}}
+unsigned _Accum ua_const = -1.0k; // expected-warning{{implicit conversion from -1.0 cannot fit within the range of values for 'unsigned _Accum'}}
+short _Accum sa_const2 = 128.0k + 128.0k; // expected-warning{{implicit conversion from 256.0 cannot fit within the range of values for 'short _Accum'}}