[Fixed Point Arithmetic] Augmented Assignment for Fixed Point Types

[leonardchan]

Summary

This patch contains the changes and tests for augmented assignments for primary fixed point types.

s_accum = 7.5hk;
s_accum2 = 2.0hk;
s_accum += s_accum2;
assert(s_accum == 9.5hk);
s_accum += 2.5k;
assert(s_accum == 12);

Logic for moving the radix was also moved to a separate function that takes into account the order between fixed point casts before shifting.

[lebedev.ri]

Hi @leonardchan.
Could you please prepend some appropriate tag to the subjects of all your differentials, so it is less hard to read through cfe-commits mails, please?

[leonardchan]

formatting

[ebevhan]

include/clang/AST/Type.h

6562

As I mentioned in another patch, this should be in ASTContext and ask TargetInfo for the information.

I'm not a fan of the term 'fbits'. The correct terminology should be the 'scaling factor'.

lib/CodeGen/CGExprScalar.cpp

1005

For future reference, please do not rely on the behavior of floating point for fixed-point operations.

1181

I think it's safer to simply do all relevant fixed-point conversion by hand here instead of falling through and relying on the semantics of other conversions.

We can provide patches for our implementation here if wanted.

1208

This case should not be needed. Perform all fixed-point conversion before the integer type check above and do an early return instead.

It's not safe to rely on the behavior of other conversions for the behavior of these.

include/clang/AST/Type.h

 
 QualType DecayedType::getPointeeType() const {
   QualType Decayed = getDecayedType();
   (void)AttributedType::stripOuterNullability(Decayed);
   return cast<PointerType>(Decayed)->getPointeeType();
 }
 
 // Return the number of fractional bits in a fixed point type.
-unsigned getFixedPointFBits(const QualType& Ty);
+unsigned getFixedPointFBits(const Type& Ty);
+
+inline unsigned getFixedPointFBits(const QualType& Ty) {
+  return getFixedPointFBits(*Ty);
+}
 
 // Return the number of integral bits in a fixed point type.
-unsigned getFixedPointIBits(const QualType& Ty);
+unsigned getFixedPointIBits(const Type& Ty);
+
+inline unsigned getFixedPointIBits(const QualType& Ty) {
+  return getFixedPointIBits(*Ty);
+}
 
 } // namespace clang
 
 #endif // LLVM_CLANG_AST_TYPE_H

lib/AST/Type.cpp

QualType::DestructionKind QualType::isDestructedTypeImpl(QualType type) {
 
   return DK_none;
 }
 
 CXXRecordDecl *MemberPointerType::getMostRecentCXXRecordDecl() const {
   return getClass()->getAsCXXRecordDecl()->getMostRecentDecl();
 }
 
-unsigned clang::getFixedPointFBits(const QualType& Ty) {
-  assert(Ty->isFixedPointType());
+unsigned clang::getFixedPointFBits(const Type& Ty) {
+  assert(Ty.isFixedPointType());
 
-  const auto *BT = Ty->getAs<BuiltinType>();
+  const auto &BT = Ty.getAs<BuiltinType>();
   switch (BT->getKind()) {
     default: llvm_unreachable("Not a fixed point type!");
     case BuiltinType::ShortAccum:
     case BuiltinType::SatShortAccum:
       return BUILTIN_SACCUM_FBIT;
 
     case BuiltinType::Accum:
     case BuiltinType::SatAccum:
case BuiltinType::SatUFract:
       return BUILTIN_UFRACT_FBIT;
 
     case BuiltinType::ULongFract:
     case BuiltinType::SatULongFract:
       return BUILTIN_ULFRACT_FBIT;
   }
 }
 
-unsigned clang::getFixedPointIBits(const QualType& Ty) {
-  assert(Ty->isFixedPointType());
+unsigned clang::getFixedPointIBits(const Type& Ty) {
+  assert(Ty.isFixedPointType());
 
-  const auto *BT = Ty->getAs<BuiltinType>();
+  const auto &BT = Ty.getAs<BuiltinType>();
   switch (BT->getKind()) {
     default: llvm_unreachable("Not a fixed point type!");
     case BuiltinType::ShortAccum:
     case BuiltinType::SatShortAccum:
       return BUILTIN_SACCUM_IBIT;
 
     case BuiltinType::Accum:
     case BuiltinType::SatAccum:

lib/CodeGen/CGExprScalar.cpp

public:
   /// Emit a conversion from the specified type to the specified destination
   /// type, both of which are LLVM scalar types.
   Value *EmitScalarConversion(Value *Src, QualType SrcTy, QualType DstTy,
                               SourceLocation Loc);
 
   Value *EmitScalarConversion(Value *Src, QualType SrcTy, QualType DstTy,
                               SourceLocation Loc, bool TreatBooleanAsSigned);
 
+  /// Emit a conversion between fixed point types by moving the radix point.
+  /// This does not take into account resizing of the underlying llvm type
+  /// which should be handled either before or after calling this function.
+  /// 
+  /// If the type is being scaled up, this method should be called after
+  /// performing an intcast. If the type is scaled down, this method should be
+  /// called before performing an intcast. This is necessary such that the
+  /// shift operations retain as much of the original data as possible before
+  /// truncation or after extension.
+  Value *EmitFixedPointRadixShift(Value *Src, QualType SrcTy, QualType DstTy);
+
   /// Emit a conversion from the specified complex type to the specified
   /// destination type, where the destination type is an LLVM scalar type.
   Value *EmitComplexToScalarConversion(CodeGenFunction::ComplexPairTy Src,
                                        QualType SrcTy, QualType DstTy,
                                        SourceLocation Loc);
 
   /// EmitNullValue - Emit a value that corresponds to null for the given type.
   Value *EmitNullValue(QualType Ty);
void ScalarExprEmitter::EmitFloatConversionCheck(
 
   llvm::Constant *StaticArgs[] = {CGF.EmitCheckSourceLocation(Loc),
                                   CGF.EmitCheckTypeDescriptor(OrigSrcType),
                                   CGF.EmitCheckTypeDescriptor(DstType)};
   CGF.EmitCheck(std::make_pair(Check, SanitizerKind::FloatCastOverflow),
                 SanitizerHandler::FloatCastOverflow, StaticArgs, OrigSrc);
 }
 
+
+/// Emit a conversion between fixed point types by moving the radix point.
+/// This does not take into account resizing of the underlying llvm type
+/// which should be handled either before or after calling this function.
+/// 
+/// If the type is being scaled up, this method should be called after
+/// performing an intcast. If the type is scaled down, this method should be
+/// called before performing an intcast. This is necessary such that the
+/// shift operations retain as much of the original data as possible before
+/// truncation or after extension.
+Value *ScalarExprEmitter::EmitFixedPointRadixShift(Value *Src, QualType SrcTy,
+                                                   QualType DstTy) {
+  assert(DstTy->isFixedPointType());
+  assert(SrcTy->isFixedPointType());
+
+  Value* Res = Src;
+
+  // Casting between fixed point types involves separating the integral and
+  // fractional bits, potentially shifting them, then joining back together.
+  unsigned dest_fbits = getFixedPointFBits(DstTy);
+  unsigned src_fbits = getFixedPointFBits(SrcTy);
+  unsigned dest_ibits = getFixedPointIBits(DstTy);
+  unsigned src_ibits = getFixedPointIBits(SrcTy);
+
+  // If the number of integral bits is decreasing, trim off any extra bits while
+  // retaining the sign.
+  if (dest_ibits < src_ibits) {
+    Res = Builder.CreateShl(Res, src_ibits - dest_ibits);
+    Res = Builder.CreateAShr(Res, src_ibits - dest_ibits);
+  }
+
+  // Move the radix. For irrational numbers, there will be loss of precision using
+  // this method when the number of fbits increases since we will be right padding
+  // zeros. Precision can still be retained if we temporarily convert to a float
+  // and perform some floating point arithmetic, though this may cost more. Enable
+  // that if #pragma FX_FULL_PRECISION is provided.
+  if (dest_fbits > src_fbits) {
+    Res = Builder.CreateShl(Res, dest_fbits - src_fbits);
+  } else if (dest_fbits < src_fbits) {
+    Res = Builder.CreateAShr(Res, src_fbits - dest_fbits);
+  }
+  return Res;
+}
+
 /// Emit a conversion from the specified type to the specified destination type,
 /// both of which are LLVM scalar types.
 Value *ScalarExprEmitter::EmitScalarConversion(Value *Src, QualType SrcType,
                                                QualType DstType,
                                                SourceLocation Loc) {
   return EmitScalarConversion(Src, SrcType, DstType, Loc, false);
 }
 
if (SrcTy->isFloatingPointTy()) {
         return Builder.CreateCall(
             CGF.CGM.getIntrinsic(llvm::Intrinsic::convert_to_fp16, SrcTy), Src);
       // If the half type is supported, just use an fptrunc.
       return Builder.CreateFPTrunc(Src, DstTy);
     }
     DstTy = CGF.FloatTy;
   }
 
+  bool WorkingOnFixedPoints = DstType->isFixedPointType() && SrcType->isFixedPointType();
+  int order = WorkingOnFixedPoints ? CGF.getContext().getFixedPointTypeOrder(DstType, SrcType) : 0;
+
+  if (WorkingOnFixedPoints && order < 0) {
+    // Casting down, so we will need to shift early as to not lose data
+    Src = EmitFixedPointRadixShift(Src, SrcType, DstType);
+  }
+
   if (isa<llvm::IntegerType>(SrcTy)) {
     bool InputSigned = (SrcType->isSignedIntegerOrEnumerationType() ||
                         SrcType->isFixedPointType());
     if (SrcType->isBooleanType() && TreatBooleanAsSigned) {
       InputSigned = true;
     }
     if (isa<llvm::IntegerType>(DstTy))
       Res = Builder.CreateIntCast(Src, DstTy, InputSigned, "conv");
     else if (InputSigned)
if (isa<llvm::IntegerType>(SrcTy)) {
     assert(SrcTy->isFloatingPointTy() && DstTy->isFloatingPointTy() &&
            "Unknown real conversion");
     if (DstTy->getTypeID() < SrcTy->getTypeID())
       Res = Builder.CreateFPTrunc(Src, DstTy, "conv");
     else
       Res = Builder.CreateFPExt(Src, DstTy, "conv");
   }
 
+  if (WorkingOnFixedPoints && order >= 0) {
+    // Casting up (or same type), so we can safely shift without losing data
+    Res = EmitFixedPointRadixShift(Res, SrcType, DstType);
+  }
+
   if (DstTy != ResTy) {
     if (CGF.getContext().getTargetInfo().useFP16ConversionIntrinsics()) {
       assert(ResTy->isIntegerTy(16) && "Only half FP requires extra conversion");
       Res = Builder.CreateCall(
         CGF.CGM.getIntrinsic(llvm::Intrinsic::convert_to_fp16, CGF.CGM.FloatTy),
         Res);
     } else {
       Res = Builder.CreateFPTrunc(Res, ResTy, "conv");
case CK_VectorSplat: {
     llvm::Type *DstTy = ConvertType(DestTy);
     Value *Elt = Visit(const_cast<Expr*>(E));
     // Splat the element across to all elements
     unsigned NumElements = DstTy->getVectorNumElements();
     return Builder.CreateVectorSplat(NumElements, Elt, "splat");
   }
 
   case CK_FixedPointCast: {
-    // Casting between fixed point types involves separating the integral and
-    // fractional bits, potentially shifting them, then joining back together.
-    assert(DestTy->isFixedPointType());
-    assert(E->getType()->isFixedPointType());
-
-    unsigned dest_fbits = getFixedPointFBits(DestTy);
-    unsigned src_fbits = getFixedPointFBits(E->getType());
-    unsigned dest_ibits = getFixedPointIBits(DestTy);
-    unsigned src_ibits = getFixedPointIBits(E->getType());
-
-    llvm::Value* result = EmitScalarConversion(Visit(E), E->getType(), DestTy, CE->getExprLoc());
-
-    // If the number of integral bits is decreasing, trim off any extra bits while
-    // retaining the sign.
-    if (dest_ibits < src_ibits) {
-      result = Builder.CreateShl(result, src_ibits - dest_ibits);
-      result = Builder.CreateAShr(result, src_ibits - dest_ibits);
-    }
-
-    // Move the radix. For irrational numbers, there will be loss of precision using
-    // this method when the number of fbits increases since we will be right padding
-    // zeros. Precision can still be retained if we temporarily convert to a float
-    // and perform some floating point arithmetic, though this may cost more. Enable
-    // that if #pragma FX_FULL_PRECISION is provided.
-    if (dest_fbits > src_fbits) {
-      result = EmitScalarConversion(result, E->getType(), DestTy, CE->getExprLoc());
-      result = Builder.CreateShl(result, dest_fbits - src_fbits);
-    } else if (dest_fbits < src_fbits) {
-      result = Builder.CreateAShr(result, src_fbits - dest_fbits);
-    }
-    return result;
+    return EmitScalarConversion(Visit(E), E->getType(), DestTy, CE->getExprLoc());
   }
 
   case CK_IntegralToFixedPoint: {
     assert(DestTy->isFixedPointType());
     assert(E->getType()->isIntegerType());
     return Builder.CreateShl(EmitScalarConversion(Visit(E), E->getType(), DestTy, CE->getExprLoc()),
                              getFixedPointFBits(DestTy), "int_to_fixed");
   }

lib/Sema/SemaExpr.cpp

case Type::STK_ObjCObjectPointer:
     }
     llvm_unreachable("Should have returned before this");
 
   case Type::STK_FixedPoint: {
     switch (DestTy->getScalarTypeKind()) {
       default: llvm_unreachable("Unable to convert from fixed point type");
       case Type::STK_Integral: llvm_unreachable("Unimplemented scalar cast from fixed point to int");  // TODO
       case Type::STK_Floating: return CK_FixedPointToFloating;
-      case Type::STK_FixedPoint: llvm_unreachable("Unimplemented scalar cast from fixed point to fixed point");  // TODO
+      case Type::STK_FixedPoint: return CK_FixedPointCast;
     }
   }
 
   case Type::STK_Bool: // casting from bool is like casting from an integer
   case Type::STK_Integral:
     switch (DestTy->getScalarTypeKind()) {
     case Type::STK_FixedPoint: llvm_unreachable("Sema::PrepareScalarCast from STK_Integral to STK_FixedPoint");  // TODO
     case Type::STK_CPointer:

test/Frontend/fixed_point_validation.c

int main(){
   assert((float)2.333hk != 2.333f);
 
   float base = 2.333f;
   float saccum_diff = abs(base - 2.333hk);
   float accum_diff = abs(base - 2.333k);
   float laccum_diff = abs(base - 2.333lk);
   assert(accum_diff < saccum_diff);
   assert(laccum_diff < accum_diff);
+
+  /**************** Auxillary assignments ***************/
+
+  s_accum = 7.5hk;
+  s_accum2 = 2.0hk;
+  s_accum += s_accum2;
+  assert(s_accum == 9.5hk);
+  s_accum += 2.5k;
+  assert(s_accum == 12);
+
+  s_accum -= s_accum2;
+  assert(s_accum == 10);
+  s_accum -= 2.5lk;
+  assert(s_accum == 7.5k);
+
+  s_accum2 = 3.0k;
+  s_accum *= s_accum2;
+  assert(s_accum == 22.5k);
+  s_accum *= 0.5r;
+  assert(s_accum == 11.25hk);
+
+  s_accum /= s_accum2;
+  assert(s_accum == 3.75k);
+  s_accum /= 0.5hr;
+  assert(s_accum == 7.5k);
+
+  s_accum <<= 3;
+  assert(s_accum == 60);
+
+  s_accum >>= 3;
+  assert(s_accum == 7.5k);
 }