[Fixed Point Arithmetic] Fixed Point to Boolean Cast

[leonardchan]

Summary

This patch is a part of https://reviews.llvm.org/D48456 in an attempt to split the casting logic up into smaller patches. This contains the code for casting from fixed point types to boolean types.

[ebevhan]

clang/lib/CodeGen/CGExprScalar.cpp

1032 ↗

(On Diff #169775)

Is this comment true? I don't think EmitFixedPointConversion does this.

Maybe I'm misinterpreting what it means.

2023 ↗

(On Diff #169775)

This comment is a duplicate of the one in EmitScalarConversion.

clang/lib/Sema/Sema.cpp

537 ↗

(On Diff #169775)

Put this on the line above directly after the case like the others.

[leonardchan]

clang/lib/CodeGen/CGExprScalar.cpp

1032 ↗

(On Diff #169775)

Ah sorry, this was my bad. I briefly forgot that we intended for overflow into the bit to be undefined behavior. In that case, this comment doesn't make sense and should be removed, but we still shouldn't have to clear/check the padding bit in this case because we should assume it's zero'd.

So this refers to handling when the padding bit needs to be cleared since if we ever have an operation that reads the whole int of an underlying unsigned type. I accidentally wrote this comment thinking that it would be cleared/zero'd at some point when we decided before that overflow into this padding was just undefined behavior. Updated the comment to reflect this.

[rjmccall]

clang/lib/AST/ExprConstant.cpp

9599 ↗

(On Diff #169863)

I know you haven't really done constant-evaluation yet, but I think you should at least be setting up operations like this correctly:

1. There should be an EvaluateFixedPoint function that evaluates an expression as a fixed-point value, just like EvaluateFloat and the others, which can be structured to use FixedPointExprEvaluator.
2. There should be a getBoolValue method on APFixedPoint.

clang/lib/CodeGen/CGExprScalar.cpp

2026 ↗

(On Diff #169863)

Why are you pushing these casts through EmitScalarConversion when the cast kind already tells you which operation you're doing?

[ebevhan]

clang/lib/CodeGen/CGExprScalar.cpp

2026 ↗

(On Diff #169863)

It could be useful to enable EmitScalarConversion to handle any of the conversions so it can be used in other contexts than expanding a cast.

[leonardchan]

clang/lib/CodeGen/CGExprScalar.cpp

2026 ↗

(On Diff #169863)

It could be useful to enable EmitScalarConversion to handle any of the conversions so it can be used in other contexts than expanding a cast.

This was also the reason I did this since EmitScalarConversion seems to get called in many places as a generic dispatch for different conversions.

[rjmccall]

LGTM.

clang/lib/CodeGen/CGExprScalar.cpp

2026 ↗

(On Diff #169863)

I think we do that in a very restricted set of places, like compound assignment operators, and honestly they should be changed so that they store the final cast kind. But I suppose I can't ask you to fight the current system.

[leonardchan]

clang/lib/AST/ExprConstant.cpp

9599 ↗

(On Diff #169863)

I will add these in another patch since this has LGTM.

include/clang/AST/OperationKinds.def

 /// CK_IntegralToFloating - Integral to floating point.
 ///    float f = i;
 CAST_OPERATION(IntegralToFloating)
 
 /// CK_FixedPointCast - Fixed point to fixed point.
 ///    (_Accum) 0.5r
 CAST_OPERATION(FixedPointCast)
 
+/// CK_FixedPointToBoolean - Fixed point to boolean.
+///    (bool) 0.5r
+CAST_OPERATION(FixedPointToBoolean)
+
 /// CK_FloatingToIntegral - Floating point to integral.  Rounds
 /// towards zero, discarding any fractional component.
 ///    (int) f
 CAST_OPERATION(FloatingToIntegral)
 
 /// CK_FloatingToBoolean - Floating point to boolean.
 ///    (bool) f
 CAST_OPERATION(FloatingToBoolean)

lib/AST/Expr.cpp

bool CastExpr::CastConsistency() const {
   case CK_IntegralToBoolean:
   case CK_FloatingToBoolean:
   case CK_MemberPointerToBoolean:
   case CK_FloatingComplexToBoolean:
   case CK_IntegralComplexToBoolean:
   case CK_LValueBitCast:            // -> bool&
   case CK_UserDefinedConversion:    // operator bool()
   case CK_BuiltinFnToFnPtr:
+  case CK_FixedPointToBoolean:
   CheckNoBasePath:
     assert(path_empty() && "Cast kind should not have a base path!");
     break;
   }
   return true;
 }
 
 const char *CastExpr::getCastKindName(CastKind CK) {

lib/AST/ExprConstant.cpp

case CK_IntegralComplexToBoolean: {
     if (!EvaluateAsBooleanCondition(SubExpr, BoolResult, Info))
       return false;
     uint64_t IntResult = BoolResult;
     if (BoolResult && E->getCastKind() == CK_BooleanToSignedIntegral)
       IntResult = (uint64_t)-1;
     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);
+  }
+
   case CK_IntegralCast: {
     if (!Visit(SubExpr))
       return false;
 
     if (!Result.isInt()) {
       // Allow casts of address-of-label differences if they are no-ops
       // or narrowing.  (The narrowing case isn't actually guaranteed to
       // be constant-evaluatable except in some narrow cases which are hard
bool ComplexExprEvaluator::VisitCastExpr(const CastExpr *E) {
   case CK_ARCExtendBlockObject:
   case CK_CopyAndAutoreleaseBlockObject:
   case CK_BuiltinFnToFnPtr:
   case CK_ZeroToOCLOpaqueType:
   case CK_NonAtomicToAtomic:
   case CK_AddressSpaceConversion:
   case CK_IntToOCLSampler:
   case CK_FixedPointCast:
+  case CK_FixedPointToBoolean:
     llvm_unreachable("invalid cast kind for complex value");
 
   case CK_LValueToRValue:
   case CK_AtomicToNonAtomic:
   case CK_NoOp:
     return ExprEvaluatorBaseTy::VisitCastExpr(E);
 
   case CK_Dependent:

lib/CodeGen/CGExpr.cpp

LValue CodeGenFunction::EmitCastLValue(const CastExpr *E) {
   case CK_ARCProduceObject:
   case CK_ARCConsumeObject:
   case CK_ARCReclaimReturnedObject:
   case CK_ARCExtendBlockObject:
   case CK_CopyAndAutoreleaseBlockObject:
   case CK_AddressSpaceConversion:
   case CK_IntToOCLSampler:
   case CK_FixedPointCast:
+  case CK_FixedPointToBoolean:
     return EmitUnsupportedLValue(E, "unexpected cast lvalue");
 
   case CK_Dependent:
     llvm_unreachable("dependent cast kind in IR gen!");
 
   case CK_BuiltinFnToFnPtr:
     llvm_unreachable("builtin functions are handled elsewhere");
 

lib/CodeGen/CGExprAgg.cpp

void AggExprEmitter::VisitCastExpr(CastExpr *E) {
   case CK_ARCReclaimReturnedObject:
   case CK_ARCExtendBlockObject:
   case CK_CopyAndAutoreleaseBlockObject:
   case CK_BuiltinFnToFnPtr:
   case CK_ZeroToOCLOpaqueType:
   case CK_AddressSpaceConversion:
   case CK_IntToOCLSampler:
   case CK_FixedPointCast:
+  case CK_FixedPointToBoolean:
     llvm_unreachable("cast kind invalid for aggregate types");
   }
 }
 
 void AggExprEmitter::VisitCallExpr(const CallExpr *E) {
   if (E->getCallReturnType(CGF.getContext())->isReferenceType()) {
     EmitAggLoadOfLValue(E);
     return;

lib/CodeGen/CGExprComplex.cpp

ComplexPairTy ComplexExprEmitter::EmitCast(CastKind CK, Expr *Op,
   case CK_ARCReclaimReturnedObject:
   case CK_ARCExtendBlockObject:
   case CK_CopyAndAutoreleaseBlockObject:
   case CK_BuiltinFnToFnPtr:
   case CK_ZeroToOCLOpaqueType:
   case CK_AddressSpaceConversion:
   case CK_IntToOCLSampler:
   case CK_FixedPointCast:
+  case CK_FixedPointToBoolean:
     llvm_unreachable("invalid cast kind for complex value");
 
   case CK_FloatingRealToComplex:
   case CK_IntegralRealToComplex:
     return EmitScalarToComplexCast(CGF.EmitScalarExpr(Op), Op->getType(),
                                    DestTy, Op->getExprLoc());
 
   case CK_FloatingComplexCast:

lib/CodeGen/CGExprConstant.cpp

llvm::Constant *VisitCastExpr(CastExpr *E, QualType destType) {
     case CK_BooleanToSignedIntegral:
     case CK_IntegralToPointer:
     case CK_IntegralToBoolean:
     case CK_IntegralToFloating:
     case CK_FloatingToIntegral:
     case CK_FloatingToBoolean:
     case CK_FloatingCast:
     case CK_FixedPointCast:
+    case CK_FixedPointToBoolean:
     case CK_ZeroToOCLOpaqueType:
       return nullptr;
     }
     llvm_unreachable("Invalid CastKind");
   }
 
   llvm::Constant *VisitCXXDefaultArgExpr(CXXDefaultArgExpr *DAE, QualType T) {
     return Visit(DAE->getExpr(), T);

lib/CodeGen/CGExprScalar.cpp

 }
 
 /// 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,
                                                ScalarConversionOpts Opts) {
-  assert(!SrcType->isFixedPointType() && !DstType->isFixedPointType() &&
-         "Use the ScalarExprEmitter::EmitFixedPoint family functions for "
-         "handling conversions involving fixed point types.");
+  // All conversions involving fixed point types should be handled by the
+  // EmitFixedPoint family functions. This is done to prevent bloating up this
+  // function more, and although fixed point numbers are represented by
+  // integers, we do not want to follow any logic that assumes they should be
+  // treated as integers.
+  // TODO(leonardchan): When necessary, add another if statement checking for
+  // conversions to fixed point types from other types.
+  if (SrcType->isFixedPointType()) {
+    if (DstType->isFixedPointType()) {
+      return EmitFixedPointConversion(Src, SrcType, DstType, Loc);
+    } else if (DstType->isBooleanType()) {
+      // We do not need to check the padding bit on unsigned types if unsigned
+      // padding is enabled because overflow into this bit is undefined
+      // behavior.
+      return Builder.CreateIsNotNull(Src);
+    }
+
+    llvm_unreachable(
+        "Unhandled scalar conversion involving a fixed point type.");
+  }
 
   QualType NoncanonicalSrcType = SrcType;
   QualType NoncanonicalDstType = DstType;
 
   SrcType = CGF.getContext().getCanonicalType(SrcType);
   DstType = CGF.getContext().getCanonicalType(DstType);
   if (SrcType == DstType) return Src;
 
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:
-    return EmitFixedPointConversion(Visit(E), E->getType(), DestTy,
-                                    CE->getExprLoc());
+    return EmitScalarConversion(Visit(E), E->getType(), DestTy,
+                                CE->getExprLoc());
+
+  case CK_FixedPointToBoolean:
+    assert(E->getType()->isFixedPointType() &&
+           "Expected src type to be fixed point type");
+    assert(DestTy->isBooleanType() && "Expected dest type to be boolean type");
+    return EmitScalarConversion(Visit(E), E->getType(), DestTy,
 
   case CK_IntegralCast: {
     ScalarConversionOpts Opts;
     if (CGF.SanOpts.hasOneOf(SanitizerKind::ImplicitIntegerTruncation)) {
       if (auto *ICE = dyn_cast<ImplicitCastExpr>(CE))
         Opts.EmitImplicitIntegerTruncationChecks = !ICE->isPartOfExplicitCast();
     }
     return EmitScalarConversion(Visit(E), E->getType(), DestTy,

lib/Edit/RewriteObjCFoundationAPI.cpp

if (const ImplicitCastExpr *ICE = dyn_cast<ImplicitCastExpr>(Arg)) {
     case CK_ZeroToOCLOpaqueType:
     case CK_IntToOCLSampler:
       return false;
 
     case CK_BooleanToSignedIntegral:
       llvm_unreachable("OpenCL-specific cast in Objective-C?");
 
     case CK_FixedPointCast:
+    case CK_FixedPointToBoolean:
       llvm_unreachable("Fixed point types are disabled for Objective-C");
     }
   }
 
   if (needsCast) {
     DiagnosticsEngine &Diags = Ctx.getDiagnostics();
     // FIXME: Use a custom category name to distinguish migration diagnostics.
     unsigned diagID = Diags.getCustomDiagID(DiagnosticsEngine::Warning,

lib/Sema/Sema.cpp

CastKind Sema::ScalarTypeToBooleanCastKind(QualType ScalarTy) {
   case Type::STK_CPointer: return CK_PointerToBoolean;
   case Type::STK_BlockPointer: return CK_PointerToBoolean;
   case Type::STK_ObjCObjectPointer: return CK_PointerToBoolean;
   case Type::STK_MemberPointer: return CK_MemberPointerToBoolean;
   case Type::STK_Integral: return CK_IntegralToBoolean;
   case Type::STK_Floating: return CK_FloatingToBoolean;
   case Type::STK_IntegralComplex: return CK_IntegralComplexToBoolean;
   case Type::STK_FloatingComplex: return CK_FloatingComplexToBoolean;
-  case Type::STK_FixedPoint:
-    llvm_unreachable("Unknown cast from FixedPoint to boolean");
+  case Type::STK_FixedPoint: return CK_FixedPointToBoolean;
   }
   llvm_unreachable("unknown scalar type kind");
 }
 
 /// Used to prune the decls of Sema's UnusedFileScopedDecls vector.
 static bool ShouldRemoveFromUnused(Sema *SemaRef, const DeclaratorDecl *D) {
   if (D->getMostRecentDecl()->isUsed())
     return true;

lib/Sema/SemaExpr.cpp

case Type::STK_ObjCObjectPointer:
     }
     llvm_unreachable("Should have returned before this");
 
   case Type::STK_FixedPoint:
     switch (DestTy->getScalarTypeKind()) {
     case Type::STK_FixedPoint:
       return CK_FixedPointCast;
     case Type::STK_Bool:
-      Diag(Src.get()->getExprLoc(),
-           diag::err_unimplemented_conversion_with_fixed_point_type)
-          << DestTy;
-      return CK_IntegralToBoolean;
+      return CK_FixedPointToBoolean;
     case Type::STK_Integral:
     case Type::STK_Floating:
     case Type::STK_IntegralComplex:
     case Type::STK_FloatingComplex:
       Diag(Src.get()->getExprLoc(),
            diag::err_unimplemented_conversion_with_fixed_point_type)
           << DestTy;
       return CK_IntegralCast;
case UO_LNot: // logical negation
 
     if (resultType->isDependentType())
       break;
     if (resultType->isScalarType() && !isScopedEnumerationType(resultType)) {
       // C99 6.5.3.3p1: ok, fallthrough;
       if (Context.getLangOpts().CPlusPlus) {
         // C++03 [expr.unary.op]p8, C++0x [expr.unary.op]p9:
         // operand contextually converted to bool.
-        if (resultType->getScalarTypeKind() == Type::STK_FixedPoint) {
-          return ExprError(
-              Diag(Input.get()->getExprLoc(),
-                   diag::err_unimplemented_conversion_with_fixed_point_type)
-              << resultType);
-        }
         Input = ImpCastExprToType(Input.get(), Context.BoolTy,
                                   ScalarTypeToBooleanCastKind(resultType));
       } else if (Context.getLangOpts().OpenCL &&
                  Context.getLangOpts().OpenCLVersion < 120) {
         // OpenCL v1.1 6.3.h: The logical operator not (!) does not
         // operate on scalar float types.
         if (!resultType->isIntegerType() && !resultType->isPointerType())
           return ExprError(Diag(OpLoc, diag::err_typecheck_unary_expr)

lib/StaticAnalyzer/Core/ExprEngineC.cpp

switch (CastE->getCastKind()) {
       case CK_IntegralComplexToFloatingComplex:
       case CK_CPointerToObjCPointerCast:
       case CK_BlockPointerToObjCPointerCast:
       case CK_AnyPointerToBlockPointerCast:
       case CK_ObjCObjectLValueCast:
       case CK_ZeroToOCLOpaqueType:
       case CK_IntToOCLSampler:
       case CK_LValueBitCast:
-      case CK_FixedPointCast: {
+      case CK_FixedPointCast:
+      case CK_FixedPointToBoolean: {
         state =
             handleLValueBitCast(state, Ex, LCtx, T, ExTy, CastE, Bldr, Pred);
         continue;
       }
       case CK_IntegralCast: {
         // Delegate to SValBuilder to process.
         SVal V = state->getSVal(Ex, LCtx);
         V = svalBuilder.evalIntegralCast(state, V, T, ExTy);

test/Frontend/fixed_point_to_bool.c

+// RUN: %clang_cc1 -ffixed-point -S -emit-llvm %s -o - | FileCheck %s
+// RUN: %clang_cc1 -ffixed-point -S -emit-llvm %s -o - -fpadding-on-unsigned-fixed-point | FileCheck %s
+
+_Bool global_b = 1.0k;  // @global_b = {{*.}}global i8 1, align 1
+_Bool global_b2 = 0.0k; // @global_b2 = {{*.}}global i8 0, align 1
+
+void func() {
+  _Accum a = 0.5k;
+  unsigned _Accum ua = 0.5uk;
+  _Bool b;
+
+  // CHECK: store i8 1, i8* %b, align 1
+  // CHECK-NEXT: store i8 0, i8* %b, align 1
+  // CHECK: store i8 1, i8* %b, align 1
+  // CHECK-NEXT: store i8 0, i8* %b, align 1
+  b = 0.5k;
+  b = 0.0k;
+  b = 0.5uk;
+  b = 0.0uk;
+
+  // CHECK-NEXT: store i8 1, i8* %b, align 1
+  // CHECK-NEXT: store i8 0, i8* %b, align 1
+  // CHECK-NEXT: store i8 1, i8* %b, align 1
+  // CHECK-NEXT: store i8 0, i8* %b, align 1
+  b = (_Bool)0.5r;
+  b = (_Bool)0.0r;
+  b = (_Bool)0.5ur;
+  b = (_Bool)0.0ur;
+
+  // CHECK-NEXT: [[ACCUM:%[0-9]+]] = load i32, i32* %a, align 4
+  // CHECK-NEXT: [[NOTZERO:%[0-9]+]] = icmp ne i32 [[ACCUM]], 0
+  // CHECK-NEXT: %frombool = zext i1 [[NOTZERO]] to i8
+  // CHECK-NEXT: store i8 %frombool, i8* %b, align 1
+  b = a;
+
+  // CHECK-NEXT: [[ACCUM:%[0-9]+]] = load i32, i32* %ua, align 4
+  // CHECK-NEXT: [[NOTZERO:%[0-9]+]] = icmp ne i32 [[ACCUM]], 0
+  // CHECK-NEXT: %frombool1 = zext i1 [[NOTZERO]] to i8
+  // CHECK-NEXT: store i8 %frombool1, i8* %b, align 1
+  b = ua;
+
+  // CHECK-NEXT: [[ACCUM:%[0-9]+]] = load i32, i32* %a, align 4
+  // CHECK-NEXT: [[NOTZERO:%[0-9]+]] = icmp ne i32 [[ACCUM]], 0
+  // CHECK-NEXT: br i1 [[NOTZERO]], label %if.then, label %if.end
+  if (a) {
+  }
+
+  // CHECK:      [[ACCUM:%[0-9]+]] = load i32, i32* %ua, align 4
+  // CHECK-NEXT: [[NOTZERO:%[0-9]+]] = icmp ne i32 [[ACCUM]], 0
+  // CHECK-NEXT: br i1 [[NOTZERO]], label %if.then{{[0-9]+}}, label %if.end{{[0-9]+}}
+  if (ua) {
+  }
+}

test/Frontend/fixed_point_unknown_conversions.c

void func() {
   accum = dc;      // expected-error{{conversion between fixed point and '_Complex double' is not yet supported}}
   accum = ic;      // expected-error{{conversion between fixed point and '_Complex int' is not yet supported}}
   accum = s;       // expected-error{{assigning to '_Accum' from incompatible type 'struct S'}}
   accum = e;       // expected-error{{conversion between fixed point and 'enum E' is not yet supported}}
   accum = ptr;     // expected-error{{assigning to '_Accum' from incompatible type 'int *'}}
   accum_ptr = ptr; // expected-warning{{incompatible pointer types assigning to '_Accum *' from 'int *'}}
   accum = i2;      // expected-error{{conversion between fixed point and 'int_t' (aka 'int') is not yet supported}}
 
-  b = accum;       // expected-error{{conversion between fixed point and '_Bool' is not yet supported}}
   c = accum;       // expected-error{{conversion between fixed point and 'char' is not yet supported}}
   i = accum;       // expected-error{{conversion between fixed point and 'int' is not yet supported}}
   f = accum;       // expected-error{{conversion between fixed point and 'float' is not yet supported}}
   d = accum;       // expected-error{{conversion between fixed point and 'double' is not yet supported}}
   dc = accum;      // expected-error{{conversion between fixed point and '_Complex double' is not yet supported}}
   ic = accum;      // expected-error{{conversion between fixed point and '_Complex int' is not yet supported}}
   s = accum;       // expected-error{{assigning to 'struct S' from incompatible type '_Accum'}}
   e = accum;       // expected-error{{conversion between fixed point and 'enum E' is not yet supported}}
   ptr = accum;     // expected-error{{assigning to 'int *' from incompatible type '_Accum'}}
   ptr = accum_ptr; // expected-warning{{incompatible pointer types assigning to 'int *' from '_Accum *'}}
   i2 = accum;      // expected-error{{conversion between fixed point and 'int' is not yet supported}}
 }