[clang] 0b9922e - [CodeGen] Emit IR for fixed-point multiplication and division.
Author: Bevin Hansson Date: 2020-04-08T14:33:04+02:00 New Revision: 0b9922e67a0b5520d76c293e9aef13a7ad4f3a8d URL: https://github.com/llvm/llvm-project/commit/0b9922e67a0b5520d76c293e9aef13a7ad4f3a8d DIFF: https://github.com/llvm/llvm-project/commit/0b9922e67a0b5520d76c293e9aef13a7ad4f3a8d.diff LOG: [CodeGen] Emit IR for fixed-point multiplication and division. Reviewers: rjmccall, leonardchan Subscribers: cfe-commits Tags: #clang Differential Revision: https://reviews.llvm.org/D73182 Added: clang/test/Frontend/fixed_point_div.c clang/test/Frontend/fixed_point_mul.c Modified: clang/lib/CodeGen/CGExprScalar.cpp Removed: diff --git a/clang/lib/CodeGen/CGExprScalar.cpp b/clang/lib/CodeGen/CGExprScalar.cpp index a0d1fe24da92..63eb4b0fe932 100644 --- a/clang/lib/CodeGen/CGExprScalar.cpp +++ b/clang/lib/CodeGen/CGExprScalar.cpp @@ -746,6 +746,8 @@ class ScalarExprEmitter Value *V = Builder.CreateFMul(Ops.LHS, Ops.RHS, "mul"); return propagateFMFlags(V, Ops); } +if (Ops.isFixedPointBinOp()) + return EmitFixedPointBinOp(Ops); return Builder.CreateMul(Ops.LHS, Ops.RHS, "mul"); } /// Create a binary op that checks for overflow. @@ -3121,6 +3123,8 @@ Value *ScalarExprEmitter::EmitDiv(const BinOpInfo &Ops) { } return Val; } + else if (Ops.isFixedPointBinOp()) +return EmitFixedPointBinOp(Ops); else if (Ops.Ty->hasUnsignedIntegerRepresentation()) return Builder.CreateUDiv(Ops.LHS, Ops.RHS, "div"); else @@ -3543,6 +3547,32 @@ Value *ScalarExprEmitter::EmitFixedPointBinOp(const BinOpInfo &op) { } break; } + case BO_Mul: { +llvm::Intrinsic::ID IID; +if (ResultFixedSema.isSaturated()) + IID = ResultFixedSema.isSigned() +? llvm::Intrinsic::smul_fix_sat +: llvm::Intrinsic::umul_fix_sat; +else + IID = ResultFixedSema.isSigned() +? llvm::Intrinsic::smul_fix +: llvm::Intrinsic::umul_fix; +Result = Builder.CreateIntrinsic(IID, {FullLHS->getType()}, +{FullLHS, FullRHS, Builder.getInt32(CommonFixedSema.getScale())}); +break; + } + case BO_Div: { +llvm::Intrinsic::ID IID; +if (ResultFixedSema.isSaturated()) + IID = ResultFixedSema.isSigned() ? llvm::Intrinsic::sdiv_fix_sat + : llvm::Intrinsic::udiv_fix_sat; +else + IID = ResultFixedSema.isSigned() ? llvm::Intrinsic::sdiv_fix + : llvm::Intrinsic::udiv_fix; +Result = Builder.CreateIntrinsic(IID, {FullLHS->getType()}, +{FullLHS, FullRHS, Builder.getInt32(CommonFixedSema.getScale())}); +break; + } case BO_LT: return CommonFixedSema.isSigned() ? Builder.CreateICmpSLT(FullLHS, FullRHS) : Builder.CreateICmpULT(FullLHS, FullRHS); @@ -3562,8 +3592,6 @@ Value *ScalarExprEmitter::EmitFixedPointBinOp(const BinOpInfo &op) { return Builder.CreateICmpEQ(FullLHS, FullRHS); case BO_NE: return Builder.CreateICmpNE(FullLHS, FullRHS); - case BO_Mul: - case BO_Div: case BO_Shl: case BO_Shr: case BO_Cmp: diff --git a/clang/test/Frontend/fixed_point_div.c b/clang/test/Frontend/fixed_point_div.c new file mode 100644 index ..ec17e082558c --- /dev/null +++ b/clang/test/Frontend/fixed_point_div.c @@ -0,0 +1,431 @@ +// 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 + +void SignedDivision() { + // CHECK-LABEL: SignedDivision + short _Accum sa; + _Accum a, b, c, d; + long _Accum la; + unsigned short _Accum usa; + unsigned _Accum ua; + unsigned long _Accum ula; + + short _Fract sf; + _Fract f; + long _Fract lf; + unsigned short _Fract usf; + unsigned _Fract uf; + unsigned long _Fract ulf; + + // Same type + // CHECK: [[TMP0:%.*]] = load i16, i16* %sa, align 2 + // CHECK-NEXT: [[TMP1:%.*]] = load i16, i16* %sa, align 2 + // CHECK-NEXT: [[TMP2:%.*]] = call i16 @llvm.sdiv.fix.i16(i16 [[TMP0]], i16 [[TMP1]], i32 7) + // CHECK-NEXT: store i16 [[TMP2]], i16* %sa, align 2 + sa = sa / sa; + + // To larger scale and larger width + // CHECK: [[TMP3:%.*]] = load i16, i16* %sa, align 2 + // CHECK-NEXT: [[TMP4:%.*]] = load i32, i32* %a, align 4 + // CHECK-NEXT: [[RESIZE:%.*]] = sext i16 [[TMP3]] to i32 + // CHECK-NEXT: [[UPSCALE:%.*]] = shl i32 [[RESIZE]], 8 + // CHECK-NEXT: [[TMP5:%.*]] = call i32 @llvm.sdiv.fix.i32(i32 [[UPSCALE]], i32 [[TMP4]], i32 15) + // CHECK-NEXT: store i32 [[TMP5]], i32* %a, align 4 + a = sa / a; + + // To same scale and smaller width + // CHECK: [[TMP6:%.*]] = load i16, i16* %sa, align 2 + /
[clang] d5d0d8e - [AST] Compress the FixedPointSemantics type better.
Author: Bevin Hansson Date: 2020-04-08T14:33:04+02:00 New Revision: d5d0d8eb7d094f8270721662882c0094fc4fdc29 URL: https://github.com/llvm/llvm-project/commit/d5d0d8eb7d094f8270721662882c0094fc4fdc29 DIFF: https://github.com/llvm/llvm-project/commit/d5d0d8eb7d094f8270721662882c0094fc4fdc29.diff LOG: [AST] Compress the FixedPointSemantics type better. Reviewers: rjmccall, leonardchan, bjope Subscribers: cfe-commits Tags: #clang Differential Revision: https://reviews.llvm.org/D73257 Added: Modified: clang/include/clang/Basic/FixedPoint.h Removed: diff --git a/clang/include/clang/Basic/FixedPoint.h b/clang/include/clang/Basic/FixedPoint.h index fd8bae62e228..55465c604a7d 100644 --- a/clang/include/clang/Basic/FixedPoint.h +++ b/clang/include/clang/Basic/FixedPoint.h @@ -75,11 +75,11 @@ class FixedPointSemantics { } private: - unsigned Width; - unsigned Scale; - bool IsSigned; - bool IsSaturated; - bool HasUnsignedPadding; + unsigned Width : 16; + unsigned Scale : 13; + unsigned IsSigned : 1; + unsigned IsSaturated: 1; + unsigned HasUnsignedPadding : 1; }; /// The APFixedPoint class works similarly to APInt/APSInt in that it is a ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] 39baaab - [CodeGen] Emit IR for fixed-point unary operators.
Author: Bevin Hansson Date: 2020-04-08T14:33:04+02:00 New Revision: 39baaabf6de4cfcbb942434084298a3f9acf5f89 URL: https://github.com/llvm/llvm-project/commit/39baaabf6de4cfcbb942434084298a3f9acf5f89 DIFF: https://github.com/llvm/llvm-project/commit/39baaabf6de4cfcbb942434084298a3f9acf5f89.diff LOG: [CodeGen] Emit IR for fixed-point unary operators. Reviewers: rjmccall, leonardchan Subscribers: cfe-commits Tags: #clang Differential Revision: https://reviews.llvm.org/D73183 Added: clang/test/Frontend/fixed_point_unary.c Modified: clang/lib/CodeGen/CGExprScalar.cpp Removed: diff --git a/clang/lib/CodeGen/CGExprScalar.cpp b/clang/lib/CodeGen/CGExprScalar.cpp index 63eb4b0fe932..1e11884e11e9 100644 --- a/clang/lib/CodeGen/CGExprScalar.cpp +++ b/clang/lib/CodeGen/CGExprScalar.cpp @@ -129,11 +129,10 @@ struct BinOpInfo { return true; } - /// Check if either operand is a fixed point type or integer type, with at - /// least one being a fixed point type. In any case, this - /// operation did not follow usual arithmetic conversion and both operands may - /// not be the same. - bool isFixedPointBinOp() const { + /// Check if at least one operand is a fixed point type. In such cases, this + /// operation did not follow usual arithmetic conversion and both operands + /// might not be of the same type. + bool isFixedPointOp() const { // We cannot simply check the result type since comparison operations return // an int. if (const auto *BinOp = dyn_cast(E)) { @@ -141,6 +140,8 @@ struct BinOpInfo { QualType RHSType = BinOp->getRHS()->getType(); return LHSType->isFixedPointType() || RHSType->isFixedPointType(); } +if (const auto *UnOp = dyn_cast(E)) + return UnOp->getSubExpr()->getType()->isFixedPointType(); return false; } }; @@ -746,7 +747,7 @@ class ScalarExprEmitter Value *V = Builder.CreateFMul(Ops.LHS, Ops.RHS, "mul"); return propagateFMFlags(V, Ops); } -if (Ops.isFixedPointBinOp()) +if (Ops.isFixedPointOp()) return EmitFixedPointBinOp(Ops); return Builder.CreateMul(Ops.LHS, Ops.RHS, "mul"); } @@ -2620,6 +2621,36 @@ ScalarExprEmitter::EmitScalarPrePostIncDec(const UnaryOperator *E, LValue LV, } } + // Fixed-point types. + } else if (type->isFixedPointType()) { +// Fixed-point types are tricky. In some cases, it isn't possible to +// represent a 1 or a -1 in the type at all. Piggyback off of +// EmitFixedPointBinOp to avoid having to reimplement saturation. +BinOpInfo Info; +Info.E = E; +Info.Ty = E->getType(); +Info.Opcode = isInc ? BO_Add : BO_Sub; +Info.LHS = value; +Info.RHS = llvm::ConstantInt::get(value->getType(), 1, false); +// If the type is signed, it's better to represent this as +(-1) or -(-1), +// since -1 is guaranteed to be representable. +if (type->isSignedFixedPointType()) { + Info.Opcode = isInc ? BO_Sub : BO_Add; + Info.RHS = Builder.CreateNeg(Info.RHS); +} +// Now, convert from our invented integer literal to the type of the unary +// op. This will upscale and saturate if necessary. This value can become +// undef in some cases. +FixedPointSemantics SrcSema = +FixedPointSemantics::GetIntegerSemantics(value->getType() + ->getScalarSizeInBits(), + /*IsSigned=*/true); +FixedPointSemantics DstSema = +CGF.getContext().getFixedPointSemantics(Info.Ty); +Info.RHS = EmitFixedPointConversion(Info.RHS, SrcSema, DstSema, +E->getExprLoc()); +value = EmitFixedPointBinOp(Info); + // Objective-C pointer types. } else { const ObjCObjectPointerType *OPT = type->castAs(); @@ -3123,7 +3154,7 @@ Value *ScalarExprEmitter::EmitDiv(const BinOpInfo &Ops) { } return Val; } - else if (Ops.isFixedPointBinOp()) + else if (Ops.isFixedPointOp()) return EmitFixedPointBinOp(Ops); else if (Ops.Ty->hasUnsignedIntegerRepresentation()) return Builder.CreateUDiv(Ops.LHS, Ops.RHS, "div"); @@ -3487,7 +3518,7 @@ Value *ScalarExprEmitter::EmitAdd(const BinOpInfo &op) { return propagateFMFlags(V, op); } - if (op.isFixedPointBinOp()) + if (op.isFixedPointOp()) return EmitFixedPointBinOp(op); return Builder.CreateAdd(op.LHS, op.RHS, "add"); @@ -3499,14 +3530,19 @@ Value *ScalarExprEmitter::EmitFixedPointBinOp(const BinOpInfo &op) { using llvm::APSInt; using llvm::ConstantInt; - const auto *BinOp = cast(op.E); - - // The result is a fixed point type and at least one of the operands is fixed - // point while the other is either fixed point or an int. This resulting type - // should be determined by Sema::handleFixedPointConversions(). + // This is either a binary operation where at least
[clang] 313461f - [CodeGen] Emit IR for compound assignment with fixed-point operands.
Author: Bevin Hansson Date: 2020-04-08T14:33:04+02:00 New Revision: 313461f6d8f91ac8abf2fa06e17b92127b050f06 URL: https://github.com/llvm/llvm-project/commit/313461f6d8f91ac8abf2fa06e17b92127b050f06 DIFF: https://github.com/llvm/llvm-project/commit/313461f6d8f91ac8abf2fa06e17b92127b050f06.diff LOG: [CodeGen] Emit IR for compound assignment with fixed-point operands. Reviewers: rjmccall, leonardchan Subscribers: cfe-commits Tags: #clang Differential Revision: https://reviews.llvm.org/D73184 Added: clang/test/Frontend/fixed_point_compound.c Modified: clang/lib/CodeGen/CGExprScalar.cpp clang/lib/Sema/SemaExpr.cpp Removed: diff --git a/clang/lib/CodeGen/CGExprScalar.cpp b/clang/lib/CodeGen/CGExprScalar.cpp index 1e11884e11e9..fa5aa702440e 100644 --- a/clang/lib/CodeGen/CGExprScalar.cpp +++ b/clang/lib/CodeGen/CGExprScalar.cpp @@ -3537,8 +3537,16 @@ Value *ScalarExprEmitter::EmitFixedPointBinOp(const BinOpInfo &op) { QualType ResultTy = op.Ty; QualType LHSTy, RHSTy; if (const auto *BinOp = dyn_cast(op.E)) { -LHSTy = BinOp->getLHS()->getType(); RHSTy = BinOp->getRHS()->getType(); +if (const auto *CAO = dyn_cast(BinOp)) { + // For compound assignment, the effective type of the LHS at this point + // is the computation LHS type, not the actual LHS type, and the final + // result type is not the type of the expression but rather the + // computation result type. + LHSTy = CAO->getComputationLHSType(); + ResultTy = CAO->getComputationResultType(); +} else + LHSTy = BinOp->getLHS()->getType(); } else if (const auto *UnOp = dyn_cast(op.E)) { LHSTy = UnOp->getSubExpr()->getType(); RHSTy = UnOp->getSubExpr()->getType(); @@ -3558,9 +3566,10 @@ Value *ScalarExprEmitter::EmitFixedPointBinOp(const BinOpInfo &op) { Value *FullRHS = EmitFixedPointConversion(RHS, RHSFixedSema, CommonFixedSema, op.E->getExprLoc()); - // Perform the actual addition. + // Perform the actual operation. Value *Result; switch (op.Opcode) { + case BO_AddAssign: case BO_Add: { if (ResultFixedSema.isSaturated()) { llvm::Intrinsic::ID IID = ResultFixedSema.isSigned() @@ -3572,6 +3581,7 @@ Value *ScalarExprEmitter::EmitFixedPointBinOp(const BinOpInfo &op) { } break; } + case BO_SubAssign: case BO_Sub: { if (ResultFixedSema.isSaturated()) { llvm::Intrinsic::ID IID = ResultFixedSema.isSigned() @@ -3583,6 +3593,7 @@ Value *ScalarExprEmitter::EmitFixedPointBinOp(const BinOpInfo &op) { } break; } + case BO_MulAssign: case BO_Mul: { llvm::Intrinsic::ID IID; if (ResultFixedSema.isSaturated()) @@ -3597,6 +3608,7 @@ Value *ScalarExprEmitter::EmitFixedPointBinOp(const BinOpInfo &op) { {FullLHS, FullRHS, Builder.getInt32(CommonFixedSema.getScale())}); break; } + case BO_DivAssign: case BO_Div: { llvm::Intrinsic::ID IID; if (ResultFixedSema.isSaturated()) @@ -3633,10 +3645,6 @@ Value *ScalarExprEmitter::EmitFixedPointBinOp(const BinOpInfo &op) { case BO_Cmp: case BO_LAnd: case BO_LOr: - case BO_MulAssign: - case BO_DivAssign: - case BO_AddAssign: - case BO_SubAssign: case BO_ShlAssign: case BO_ShrAssign: llvm_unreachable("Found unimplemented fixed point binary operation"); diff --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp index 27cc4eb0c159..5c6b9471da11 100644 --- a/clang/lib/Sema/SemaExpr.cpp +++ b/clang/lib/Sema/SemaExpr.cpp @@ -13630,6 +13630,14 @@ ExprResult Sema::CreateBuiltinBinOp(SourceLocation OpLoc, if (ResultTy.isNull() || LHS.isInvalid() || RHS.isInvalid()) return ExprError(); + // The LHS is not converted to the result type for fixed-point compound + // assignment as the common type is computed on demand. Reset the CompLHSTy + // to the LHS type we would have gotten after unary conversions. + if (!CompLHSTy.isNull() && + (LHS.get()->getType()->isFixedPointType() || + RHS.get()->getType()->isFixedPointType())) +CompLHSTy = UsualUnaryConversions(LHS.get()).get()->getType(); + if (ResultTy->isRealFloatingType() && (getLangOpts().getFPRoundingMode() != LangOptions::FPR_ToNearest || getLangOpts().getFPExceptionMode() != LangOptions::FPE_Ignore)) diff --git a/clang/test/Frontend/fixed_point_compound.c b/clang/test/Frontend/fixed_point_compound.c new file mode 100644 index ..ee7700d01ef5 --- /dev/null +++ b/clang/test/Frontend/fixed_point_compound.c @@ -0,0 +1,374 @@ +// 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 + +short _Fract shf; +_Accum a; +unsigned _Fract uf; +unsigned long _Accum ula; + +_Sat short _Fract
[clang] e4ca64f - [Fixed Point] Add triples to test cases.
Author: Bevin Hansson Date: 2020-04-08T16:31:36+02:00 New Revision: e4ca64f1ae8a3743a9bea4926e1e7889c5b02525 URL: https://github.com/llvm/llvm-project/commit/e4ca64f1ae8a3743a9bea4926e1e7889c5b02525 DIFF: https://github.com/llvm/llvm-project/commit/e4ca64f1ae8a3743a9bea4926e1e7889c5b02525.diff LOG: [Fixed Point] Add triples to test cases. This was causing some test failures. Added: Modified: clang/test/Frontend/fixed_point_compound.c clang/test/Frontend/fixed_point_unary.c Removed: diff --git a/clang/test/Frontend/fixed_point_compound.c b/clang/test/Frontend/fixed_point_compound.c index ee7700d01ef5..d470d5c22e80 100644 --- a/clang/test/Frontend/fixed_point_compound.c +++ b/clang/test/Frontend/fixed_point_compound.c @@ -1,5 +1,5 @@ -// 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 short _Fract shf; _Accum a; diff --git a/clang/test/Frontend/fixed_point_unary.c b/clang/test/Frontend/fixed_point_unary.c index 79af819fad8a..6b2a572dd821 100644 --- a/clang/test/Frontend/fixed_point_unary.c +++ b/clang/test/Frontend/fixed_point_unary.c @@ -1,5 +1,5 @@ -// 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 -triple x86_64-unknown-linux-gnu -ffixed-point -S -emit-llvm %s -o - | FileCheck %s --check-prefixes=CHECK,SIGNED +// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -ffixed-point -fpadding-on-unsigned-fixed-point -S -emit-llvm %s -o - | FileCheck %s --check-prefixes=CHECK,UNSIGNED _Accum a; _Fract f; ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] bd50cf9 - Fix indentation in FixedPoint.h. NFC.
Author: Bevin Hansson Date: 2020-07-06T10:57:21+02:00 New Revision: bd50cf905fa7c0c7caa134301c6ca0658c81eeb1 URL: https://github.com/llvm/llvm-project/commit/bd50cf905fa7c0c7caa134301c6ca0658c81eeb1 DIFF: https://github.com/llvm/llvm-project/commit/bd50cf905fa7c0c7caa134301c6ca0658c81eeb1.diff LOG: Fix indentation in FixedPoint.h. NFC. Added: Modified: clang/include/clang/Basic/FixedPoint.h Removed: diff --git a/clang/include/clang/Basic/FixedPoint.h b/clang/include/clang/Basic/FixedPoint.h index 8937bccb0edb..0d181f30907f 100644 --- a/clang/include/clang/Basic/FixedPoint.h +++ b/clang/include/clang/Basic/FixedPoint.h @@ -93,52 +93,52 @@ class FixedPointSemantics { /// point types and should eventually be moved to LLVM if fixed point types gain /// native IR support. class APFixedPoint { - public: - APFixedPoint(const llvm::APInt &Val, const FixedPointSemantics &Sema) - : Val(Val, !Sema.isSigned()), Sema(Sema) { - assert(Val.getBitWidth() == Sema.getWidth() && -"The value should have a bit width that matches the Sema width"); - } - - APFixedPoint(uint64_t Val, const FixedPointSemantics &Sema) - : APFixedPoint(llvm::APInt(Sema.getWidth(), Val, Sema.isSigned()), - Sema) {} - - // Zero initialization. - APFixedPoint(const FixedPointSemantics &Sema) : APFixedPoint(0, Sema) {} - - llvm::APSInt getValue() const { return llvm::APSInt(Val, !Sema.isSigned()); } - inline unsigned getWidth() const { return Sema.getWidth(); } - 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. 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 of both input values. See convert() for an - // explanation of the Overflow parameter. - APFixedPoint add(const APFixedPoint &Other, bool *Overflow = nullptr) const; - APFixedPoint sub(const APFixedPoint &Other, bool *Overflow = nullptr) const; - APFixedPoint mul(const APFixedPoint &Other, bool *Overflow = nullptr) const; - APFixedPoint div(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); - } +public: + APFixedPoint(const llvm::APInt &Val, const FixedPointSemantics &Sema) + : Val(Val, !Sema.isSigned()), Sema(Sema) { +assert(Val.getBitWidth() == Sema.getWidth() && + "The value should have a bit width that matches the Sema width"); + } + + APFixedPoint(uint64_t Val, const FixedPointSemantics &Sema) + : APFixedPoint(llvm::APInt(Sema.getWidth(), Val, Sema.isSigned()), + Sema) {} + + // Zero initialization. + APFixedPoint(const FixedPointSemantics &Sema) : APFixedPoint(0, Sema) {} + + llvm::APSInt getValue() const { return llvm::APSInt(Val, !Sema.isSigned()); } + inline unsigned getWidth() const { return Sema.getWidth(); } + 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. 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 of both input values. See convert() for an + // explanation of the Overflow parameter. + APFixedPoint add(const APFixedPoint &Other, bool *Overflow = nullptr) const; + APFixedPoint sub(const APFixedPoint
[clang] 94e8ec6 - [AST] Add fixed-point division constant evaluation.
Author: Bevin Hansson Date: 2020-06-26T13:38:11+02:00 New Revision: 94e8ec631dda98639930c3fcf6b84148cd58cd59 URL: https://github.com/llvm/llvm-project/commit/94e8ec631dda98639930c3fcf6b84148cd58cd59 DIFF: https://github.com/llvm/llvm-project/commit/94e8ec631dda98639930c3fcf6b84148cd58cd59.diff LOG: [AST] Add fixed-point division constant evaluation. Reviewers: rjmccall, leonardchan, bjope Subscribers: cfe-commits Tags: #clang Differential Revision: https://reviews.llvm.org/D73187 Added: Modified: clang/include/clang/Basic/FixedPoint.h clang/lib/AST/ExprConstant.cpp clang/lib/Basic/FixedPoint.cpp clang/test/Frontend/fixed_point_div.c Removed: diff --git a/clang/include/clang/Basic/FixedPoint.h b/clang/include/clang/Basic/FixedPoint.h index 2566169cfd23..8937bccb0edb 100644 --- a/clang/include/clang/Basic/FixedPoint.h +++ b/clang/include/clang/Basic/FixedPoint.h @@ -130,6 +130,7 @@ class APFixedPoint { APFixedPoint add(const APFixedPoint &Other, bool *Overflow = nullptr) const; APFixedPoint sub(const APFixedPoint &Other, bool *Overflow = nullptr) const; APFixedPoint mul(const APFixedPoint &Other, bool *Overflow = nullptr) const; + APFixedPoint div(const APFixedPoint &Other, bool *Overflow = nullptr) const; /// Perform a unary negation (-X) on this fixed point type, taking into /// account saturation if applicable. diff --git a/clang/lib/AST/ExprConstant.cpp b/clang/lib/AST/ExprConstant.cpp index 35aeb6f8f089..b55499fd5075 100644 --- a/clang/lib/AST/ExprConstant.cpp +++ b/clang/lib/AST/ExprConstant.cpp @@ -12948,6 +12948,15 @@ bool FixedPointExprEvaluator::VisitBinaryOperator(const BinaryOperator *E) { return false; return Success(Result, E); } + case BO_Div: { +bool AddOverflow, ConversionOverflow; +APFixedPoint Result = LHSFX.div(RHSFX, &AddOverflow) + .convert(ResultFXSema, &ConversionOverflow); +if ((AddOverflow || ConversionOverflow) && +!HandleOverflow(Info, E, Result, E->getType())) + return false; +return Success(Result, E); + } default: return false; } diff --git a/clang/lib/Basic/FixedPoint.cpp b/clang/lib/Basic/FixedPoint.cpp index 21fa1c00aa83..e8db43c6fe90 100644 --- a/clang/lib/Basic/FixedPoint.cpp +++ b/clang/lib/Basic/FixedPoint.cpp @@ -254,6 +254,61 @@ APFixedPoint APFixedPoint::mul(const APFixedPoint &Other, CommonFXSema); } +APFixedPoint APFixedPoint::div(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; + + // Widen the LHS and RHS so we can perform a full division. + unsigned Wide = CommonFXSema.getWidth() * 2; + if (CommonFXSema.isSigned()) { +ThisVal = ThisVal.sextOrSelf(Wide); +OtherVal = OtherVal.sextOrSelf(Wide); + } else { +ThisVal = ThisVal.zextOrSelf(Wide); +OtherVal = OtherVal.zextOrSelf(Wide); + } + + // Upscale to compensate for the loss of precision from division, and + // perform the full division. + ThisVal = ThisVal.shl(CommonFXSema.getScale()); + llvm::APSInt Result; + if (CommonFXSema.isSigned()) { +llvm::APInt Rem; +llvm::APInt::sdivrem(ThisVal, OtherVal, Result, Rem); +// If the quotient is negative and the remainder is nonzero, round +// towards negative infinity by subtracting epsilon from the result. +if (Result.isNegative() && !Rem.isNullValue()) + Result = Result - 1; + } else +Result = ThisVal.udiv(OtherVal); + Result.setIsSigned(CommonFXSema.isSigned()); + + // If our result lies outside of the representative range of the common + // semantic, we either have overflow or saturation. + llvm::APSInt Max = APFixedPoint::getMax(CommonFXSema).getValue() + .extOrTrunc(Wide); + llvm::APSInt Min = APFixedPoint::getMin(CommonFXSema).getValue() + .extOrTrunc(Wide); + if (CommonFXSema.isSaturated()) { +if (Result < Min) + Result = Min; +else if (Result > Max) + Result = Max; + } else +Overflowed = Result < Min || Result > Max; + + if (Overflow) +*Overflow = Overflowed; + + return APFixedPoint(Result.sextOrTrunc(CommonFXSema.getWidth()), + CommonFXSema); +} + void APFixedPoint::toString(llvm::SmallVectorImpl &Str) const { llvm::APSInt Val = getValue(); unsigned Scale = getScale(); diff --git a/clang/test/Frontend/fixed_point_div.c b/clang/test/Frontend/fixed_point_div.c index ec17e082558c..28fa5e0e9cbc 100644 --- a/clang/test/Frontend/fixed
[clang] da2f852 - [AST] Fix certain consteval assignment and comma operator issues with fixed-point types.
Author: Bevin Hansson Date: 2020-06-26T13:38:11+02:00 New Revision: da2f852e1913a16a1c6940ce3d3e47158ae5ba0e URL: https://github.com/llvm/llvm-project/commit/da2f852e1913a16a1c6940ce3d3e47158ae5ba0e DIFF: https://github.com/llvm/llvm-project/commit/da2f852e1913a16a1c6940ce3d3e47158ae5ba0e.diff LOG: [AST] Fix certain consteval assignment and comma operator issues with fixed-point types. Summary: Assignment and comma operators for fixed-point types were being constevaled as other binary operators, but they need special treatment. Reviewers: rjmccall, leonardchan, bjope Subscribers: cfe-commits Tags: #clang Differential Revision: https://reviews.llvm.org/D73189 Added: clang/test/Frontend/fixed_point_crash.c Modified: clang/lib/AST/ExprConstant.cpp Removed: diff --git a/clang/lib/AST/ExprConstant.cpp b/clang/lib/AST/ExprConstant.cpp index 35b5bad677a0..ebe09c99429f 100644 --- a/clang/lib/AST/ExprConstant.cpp +++ b/clang/lib/AST/ExprConstant.cpp @@ -12920,6 +12920,9 @@ bool FixedPointExprEvaluator::VisitCastExpr(const CastExpr *E) { } bool FixedPointExprEvaluator::VisitBinaryOperator(const BinaryOperator *E) { + if (E->isPtrMemOp() || E->isAssignmentOp() || E->getOpcode() == BO_Comma) +return ExprEvaluatorBaseTy::VisitBinaryOperator(E); + const Expr *LHS = E->getLHS(); const Expr *RHS = E->getRHS(); FixedPointSemantics ResultFXSema = diff --git a/clang/test/Frontend/fixed_point_crash.c b/clang/test/Frontend/fixed_point_crash.c new file mode 100644 index ..12dc1944f018 --- /dev/null +++ b/clang/test/Frontend/fixed_point_crash.c @@ -0,0 +1,20 @@ +// RUN: %clang_cc1 -verify -ffixed-point %s + +union a { + _Accum x; + int i; +}; + +int fn1() { + union a m; + m.x = 5.6k; + return m.i; +} + +int fn2() { + union a m; + m.x = 7, 5.6k; // expected-warning {{expression result unused}} + return m.x, m.i; // expected-warning {{expression result unused}} +} + +_Accum acc = (0.5r, 6.9k); // expected-warning {{expression result unused}} ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] 53f5c8b - [AST] Add fixed-point multiplication constant evaluation.
Author: Bevin Hansson Date: 2020-06-26T13:38:11+02:00 New Revision: 53f5c8b4a14c2ef89d44062a3a045be064bdc0e7 URL: https://github.com/llvm/llvm-project/commit/53f5c8b4a14c2ef89d44062a3a045be064bdc0e7 DIFF: https://github.com/llvm/llvm-project/commit/53f5c8b4a14c2ef89d44062a3a045be064bdc0e7.diff LOG: [AST] Add fixed-point multiplication constant evaluation. Reviewers: rjmccall, leonardchan, bjope Subscribers: cfe-commits Tags: #clang Differential Revision: https://reviews.llvm.org/D73186 Added: Modified: clang/include/clang/Basic/FixedPoint.h clang/lib/AST/ExprConstant.cpp clang/lib/Basic/FixedPoint.cpp clang/test/Frontend/fixed_point_mul.c Removed: diff --git a/clang/include/clang/Basic/FixedPoint.h b/clang/include/clang/Basic/FixedPoint.h index 1f3251982325..2566169cfd23 100644 --- a/clang/include/clang/Basic/FixedPoint.h +++ b/clang/include/clang/Basic/FixedPoint.h @@ -129,6 +129,7 @@ class APFixedPoint { // explanation of the Overflow parameter. APFixedPoint add(const APFixedPoint &Other, bool *Overflow = nullptr) const; APFixedPoint sub(const APFixedPoint &Other, bool *Overflow = nullptr) const; + APFixedPoint mul(const APFixedPoint &Other, bool *Overflow = nullptr) const; /// Perform a unary negation (-X) on this fixed point type, taking into /// account saturation if applicable. diff --git a/clang/lib/AST/ExprConstant.cpp b/clang/lib/AST/ExprConstant.cpp index 5cdeb3cd0e67..35aeb6f8f089 100644 --- a/clang/lib/AST/ExprConstant.cpp +++ b/clang/lib/AST/ExprConstant.cpp @@ -12939,6 +12939,15 @@ bool FixedPointExprEvaluator::VisitBinaryOperator(const BinaryOperator *E) { return false; return Success(Result, E); } + case BO_Mul: { +bool AddOverflow, ConversionOverflow; +APFixedPoint Result = LHSFX.mul(RHSFX, &AddOverflow) + .convert(ResultFXSema, &ConversionOverflow); +if ((AddOverflow || ConversionOverflow) && +!HandleOverflow(Info, E, Result, E->getType())) + return false; +return Success(Result, E); + } default: return false; } diff --git a/clang/lib/Basic/FixedPoint.cpp b/clang/lib/Basic/FixedPoint.cpp index 29d532f49561..21fa1c00aa83 100644 --- a/clang/lib/Basic/FixedPoint.cpp +++ b/clang/lib/Basic/FixedPoint.cpp @@ -197,6 +197,63 @@ APFixedPoint APFixedPoint::sub(const APFixedPoint &Other, return APFixedPoint(Result, CommonFXSema); } +APFixedPoint APFixedPoint::mul(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; + + // Widen the LHS and RHS so we can perform a full multiplication. + unsigned Wide = CommonFXSema.getWidth() * 2; + if (CommonFXSema.isSigned()) { +ThisVal = ThisVal.sextOrSelf(Wide); +OtherVal = OtherVal.sextOrSelf(Wide); + } else { +ThisVal = ThisVal.zextOrSelf(Wide); +OtherVal = OtherVal.zextOrSelf(Wide); + } + + // Perform the full multiplication and downscale to get the same scale. + // + // Note that the right shifts here perform an implicit downwards rounding. + // This rounding could discard bits that would technically place the result + // outside the representable range. We interpret the spec as allowing us to + // perform the rounding step first, avoiding the overflow case that would + // arise. + llvm::APSInt Result; + if (CommonFXSema.isSigned()) +Result = ThisVal.smul_ov(OtherVal, Overflowed) +.ashr(CommonFXSema.getScale()); + else +Result = ThisVal.umul_ov(OtherVal, Overflowed) +.lshr(CommonFXSema.getScale()); + assert(!Overflowed && "Full multiplication cannot overflow!"); + Result.setIsSigned(CommonFXSema.isSigned()); + + // If our result lies outside of the representative range of the common + // semantic, we either have overflow or saturation. + llvm::APSInt Max = APFixedPoint::getMax(CommonFXSema).getValue() + .extOrTrunc(Wide); + llvm::APSInt Min = APFixedPoint::getMin(CommonFXSema).getValue() + .extOrTrunc(Wide); + if (CommonFXSema.isSaturated()) { +if (Result < Min) + Result = Min; +else if (Result > Max) + Result = Max; + } else +Overflowed = Result < Min || Result > Max; + + if (Overflow) +*Overflow = Overflowed; + + return APFixedPoint(Result.sextOrTrunc(CommonFXSema.getWidth()), + CommonFXSema); +} + void APFixedPoint::toString(llvm::SmallVectorImpl &Str) const { llvm::APSInt Val = getValue(); unsigned Scale = getScale(); di
[clang] 474177c - [AST] Improve overflow diagnostics for fixed-point constant evaluation.
Author: Bevin Hansson Date: 2020-06-26T13:38:11+02:00 New Revision: 474177c05381a6eb2971e67753481b5efc78d848 URL: https://github.com/llvm/llvm-project/commit/474177c05381a6eb2971e67753481b5efc78d848 DIFF: https://github.com/llvm/llvm-project/commit/474177c05381a6eb2971e67753481b5efc78d848.diff LOG: [AST] Improve overflow diagnostics for fixed-point constant evaluation. Summary: Diagnostics for overflow were not being produced for fixed-point evaluation. This patch refactors a bit of the evaluator and adds a proper diagnostic for these cases. Reviewers: rjmccall, leonardchan, bjope Subscribers: cfe-commits Tags: #clang Differential Revision: https://reviews.llvm.org/D73188 Added: Modified: clang/include/clang/Basic/DiagnosticASTKinds.td clang/lib/AST/ExprConstant.cpp clang/test/Frontend/fixed_point_errors.c Removed: diff --git a/clang/include/clang/Basic/DiagnosticASTKinds.td b/clang/include/clang/Basic/DiagnosticASTKinds.td index 75c4d0e0a139..905e3158cf40 100644 --- a/clang/include/clang/Basic/DiagnosticASTKinds.td +++ b/clang/include/clang/Basic/DiagnosticASTKinds.td @@ -346,6 +346,9 @@ def err_experimental_clang_interp_failed : Error< def warn_integer_constant_overflow : Warning< "overflow in expression; result is %0 with type %1">, InGroup>; +def warn_fixedpoint_constant_overflow : Warning< + "overflow in expression; result is %0 with type %1">, + InGroup>; // This is a temporary diagnostic, and shall be removed once our // implementation is complete, and like the preceding constexpr notes belongs diff --git a/clang/lib/AST/ExprConstant.cpp b/clang/lib/AST/ExprConstant.cpp index b55499fd5075..35b5bad677a0 100644 --- a/clang/lib/AST/ExprConstant.cpp +++ b/clang/lib/AST/ExprConstant.cpp @@ -12881,8 +12881,14 @@ bool FixedPointExprEvaluator::VisitCastExpr(const CastExpr *E) { return false; bool Overflowed; APFixedPoint Result = Src.convert(DestFXSema, &Overflowed); -if (Overflowed && !HandleOverflow(Info, E, Result, DestType)) - return false; +if (Overflowed) { + if (Info.checkingForUndefinedBehavior()) +Info.Ctx.getDiagnostics().Report(E->getExprLoc(), + diag::warn_fixedpoint_constant_overflow) + << Result.toString() << E->getType(); + else if (!HandleOverflow(Info, E, Result, E->getType())) +return false; +} return Success(Result, E); } case CK_IntegralToFixedPoint: { @@ -12894,8 +12900,14 @@ bool FixedPointExprEvaluator::VisitCastExpr(const CastExpr *E) { APFixedPoint IntResult = APFixedPoint::getFromIntValue( Src, Info.Ctx.getFixedPointSemantics(DestType), &Overflowed); -if (Overflowed && !HandleOverflow(Info, E, IntResult, DestType)) - return false; +if (Overflowed) { + if (Info.checkingForUndefinedBehavior()) +Info.Ctx.getDiagnostics().Report(E->getExprLoc(), + diag::warn_fixedpoint_constant_overflow) + << IntResult.toString() << E->getType(); + else if (!HandleOverflow(Info, E, IntResult, E->getType())) +return false; +} return Success(IntResult, E); } @@ -12920,47 +12932,41 @@ bool FixedPointExprEvaluator::VisitBinaryOperator(const BinaryOperator *E) { if (!EvaluateFixedPointOrInteger(RHS, RHSFX, Info)) return false; + bool OpOverflow = false, ConversionOverflow = false; + APFixedPoint Result(LHSFX.getSemantics()); 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); +Result = LHSFX.add(RHSFX, &OpOverflow) + .convert(ResultFXSema, &ConversionOverflow); +break; } case BO_Sub: { -bool AddOverflow, ConversionOverflow; -APFixedPoint Result = LHSFX.sub(RHSFX, &AddOverflow) - .convert(ResultFXSema, &ConversionOverflow); -if ((AddOverflow || ConversionOverflow) && -!HandleOverflow(Info, E, Result, E->getType())) - return false; -return Success(Result, E); +Result = LHSFX.sub(RHSFX, &OpOverflow) + .convert(ResultFXSema, &ConversionOverflow); +break; } case BO_Mul: { -bool AddOverflow, ConversionOverflow; -APFixedPoint Result = LHSFX.mul(RHSFX, &AddOverflow) - .convert(ResultFXSema, &ConversionOverflow); -if ((AddOverflow || ConversionOverflow) && -!HandleOverflow(Info, E, Result, E->getType())) - return false; -return Success(Result, E); +Result = LHSFX.mul(RHSFX, &OpOverflow) + .convert(ResultFXSem
[clang] eccf7fc - [AST] Add fixed-point subtraction constant evaluation.
Author: Bevin Hansson Date: 2020-06-26T13:38:11+02:00 New Revision: eccf7fc7b31a00823a0fe6c782ef312b3ba743c4 URL: https://github.com/llvm/llvm-project/commit/eccf7fc7b31a00823a0fe6c782ef312b3ba743c4 DIFF: https://github.com/llvm/llvm-project/commit/eccf7fc7b31a00823a0fe6c782ef312b3ba743c4.diff LOG: [AST] Add fixed-point subtraction constant evaluation. Reviewers: rjmccall, leonardchan Subscribers: cfe-commits Tags: #clang Differential Revision: https://reviews.llvm.org/D73185 Added: Modified: clang/include/clang/Basic/FixedPoint.h clang/lib/AST/ExprConstant.cpp clang/lib/Basic/FixedPoint.cpp clang/test/Frontend/fixed_point_sub.c Removed: diff --git a/clang/include/clang/Basic/FixedPoint.h b/clang/include/clang/Basic/FixedPoint.h index 55465c604a7d..1f3251982325 100644 --- a/clang/include/clang/Basic/FixedPoint.h +++ b/clang/include/clang/Basic/FixedPoint.h @@ -28,8 +28,8 @@ class QualType; /// The fixed point semantics work similarly to llvm::fltSemantics. The width /// specifies the whole bit width of the underlying scaled integer (with padding /// if any). The scale represents the number of fractional bits in this type. -/// When HasUnsignedPadding is true and this type is signed, the first bit -/// in the value this represents is treaded as padding. +/// When HasUnsignedPadding is true and this type is unsigned, the first bit +/// in the value this represents is treated as padding. class FixedPointSemantics { public: FixedPointSemantics(unsigned Width, unsigned Scale, bool IsSigned, @@ -125,9 +125,10 @@ class APFixedPoint { // 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 + // the precision and ranges of both input values. See convert() for an // explanation of the Overflow parameter. APFixedPoint add(const APFixedPoint &Other, bool *Overflow = nullptr) const; + APFixedPoint sub(const APFixedPoint &Other, bool *Overflow = nullptr) const; /// Perform a unary negation (-X) on this fixed point type, taking into /// account saturation if applicable. diff --git a/clang/lib/AST/ExprConstant.cpp b/clang/lib/AST/ExprConstant.cpp index afae020f3dd2..5cdeb3cd0e67 100644 --- a/clang/lib/AST/ExprConstant.cpp +++ b/clang/lib/AST/ExprConstant.cpp @@ -12924,7 +12924,16 @@ bool FixedPointExprEvaluator::VisitBinaryOperator(const BinaryOperator *E) { case BO_Add: { bool AddOverflow, ConversionOverflow; APFixedPoint Result = LHSFX.add(RHSFX, &AddOverflow) - .convert(ResultFXSema, &ConversionOverflow); + .convert(ResultFXSema, &ConversionOverflow); +if ((AddOverflow || ConversionOverflow) && +!HandleOverflow(Info, E, Result, E->getType())) + return false; +return Success(Result, E); + } + case BO_Sub: { +bool AddOverflow, ConversionOverflow; +APFixedPoint Result = LHSFX.sub(RHSFX, &AddOverflow) + .convert(ResultFXSema, &ConversionOverflow); if ((AddOverflow || ConversionOverflow) && !HandleOverflow(Info, E, Result, E->getType())) return false; diff --git a/clang/lib/Basic/FixedPoint.cpp b/clang/lib/Basic/FixedPoint.cpp index 05600dfc6d21..29d532f49561 100644 --- a/clang/lib/Basic/FixedPoint.cpp +++ b/clang/lib/Basic/FixedPoint.cpp @@ -173,6 +173,30 @@ APFixedPoint APFixedPoint::add(const APFixedPoint &Other, return APFixedPoint(Result, CommonFXSema); } +APFixedPoint APFixedPoint::sub(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.ssub_sat(OtherVal) + : ThisVal.usub_sat(OtherVal); + } else { +Result = ThisVal.isSigned() ? ThisVal.ssub_ov(OtherVal, Overflowed) +: ThisVal.usub_ov(OtherVal, Overflowed); + } + + if (Overflow) +*Overflow = Overflowed; + + return APFixedPoint(Result, CommonFXSema); +} + void APFixedPoint::toString(llvm::SmallVectorImpl &Str) const { llvm::APSInt Val = getValue(); unsigned Scale = getScale(); diff --git a/clang/test/Frontend/fixed_point_sub.c b/clang/test/Frontend/fixed_point_sub.c index 59b2e0a43aed..1e449537ae23 100644 --- a/clang/test/Frontend/fixed_point_sub.c +++ b/clang/test/Frontend/fixed_point_sub.c @@ -1,6 +1,55 @@ // RUN: %clang_cc1 -ff
[clang] fefa34f - [CodeGen] Use the common semantic for fixed-point codegen, not the result semantic.
Author: Bevin Hansson Date: 2020-06-29T16:22:29+02:00 New Revision: fefa34faf551d10967cf2547003f2dd1b2efa887 URL: https://github.com/llvm/llvm-project/commit/fefa34faf551d10967cf2547003f2dd1b2efa887 DIFF: https://github.com/llvm/llvm-project/commit/fefa34faf551d10967cf2547003f2dd1b2efa887.diff LOG: [CodeGen] Use the common semantic for fixed-point codegen, not the result semantic. Summary: Using the result semantic is wrong in some cases, such as unsigned fixed-point + signed integer. In this case, the result semantic is unsigned and the common semantic is signed. Reviewers: leonardchan Subscribers: cfe-commits Tags: #clang Differential Revision: https://reviews.llvm.org/D82662 Added: Modified: clang/lib/CodeGen/CGExprScalar.cpp clang/test/Frontend/fixed_point_add.c clang/test/Frontend/fixed_point_div.c clang/test/Frontend/fixed_point_mul.c clang/test/Frontend/fixed_point_sub.c Removed: diff --git a/clang/lib/CodeGen/CGExprScalar.cpp b/clang/lib/CodeGen/CGExprScalar.cpp index 1bbfc27403ed..922aa95150ce 100644 --- a/clang/lib/CodeGen/CGExprScalar.cpp +++ b/clang/lib/CodeGen/CGExprScalar.cpp @@ -3608,8 +3608,8 @@ Value *ScalarExprEmitter::EmitFixedPointBinOp(const BinOpInfo &op) { switch (op.Opcode) { case BO_AddAssign: case BO_Add: { -if (ResultFixedSema.isSaturated()) { - llvm::Intrinsic::ID IID = ResultFixedSema.isSigned() +if (CommonFixedSema.isSaturated()) { + llvm::Intrinsic::ID IID = CommonFixedSema.isSigned() ? llvm::Intrinsic::sadd_sat : llvm::Intrinsic::uadd_sat; Result = Builder.CreateBinaryIntrinsic(IID, FullLHS, FullRHS); @@ -3620,8 +3620,8 @@ Value *ScalarExprEmitter::EmitFixedPointBinOp(const BinOpInfo &op) { } case BO_SubAssign: case BO_Sub: { -if (ResultFixedSema.isSaturated()) { - llvm::Intrinsic::ID IID = ResultFixedSema.isSigned() +if (CommonFixedSema.isSaturated()) { + llvm::Intrinsic::ID IID = CommonFixedSema.isSigned() ? llvm::Intrinsic::ssub_sat : llvm::Intrinsic::usub_sat; Result = Builder.CreateBinaryIntrinsic(IID, FullLHS, FullRHS); @@ -3633,14 +3633,12 @@ Value *ScalarExprEmitter::EmitFixedPointBinOp(const BinOpInfo &op) { case BO_MulAssign: case BO_Mul: { llvm::Intrinsic::ID IID; -if (ResultFixedSema.isSaturated()) - IID = ResultFixedSema.isSigned() -? llvm::Intrinsic::smul_fix_sat -: llvm::Intrinsic::umul_fix_sat; +if (CommonFixedSema.isSaturated()) + IID = CommonFixedSema.isSigned() ? llvm::Intrinsic::smul_fix_sat + : llvm::Intrinsic::umul_fix_sat; else - IID = ResultFixedSema.isSigned() -? llvm::Intrinsic::smul_fix -: llvm::Intrinsic::umul_fix; + IID = CommonFixedSema.isSigned() ? llvm::Intrinsic::smul_fix + : llvm::Intrinsic::umul_fix; Result = Builder.CreateIntrinsic(IID, {FullLHS->getType()}, {FullLHS, FullRHS, Builder.getInt32(CommonFixedSema.getScale())}); break; @@ -3648,11 +3646,11 @@ Value *ScalarExprEmitter::EmitFixedPointBinOp(const BinOpInfo &op) { case BO_DivAssign: case BO_Div: { llvm::Intrinsic::ID IID; -if (ResultFixedSema.isSaturated()) - IID = ResultFixedSema.isSigned() ? llvm::Intrinsic::sdiv_fix_sat +if (CommonFixedSema.isSaturated()) + IID = CommonFixedSema.isSigned() ? llvm::Intrinsic::sdiv_fix_sat : llvm::Intrinsic::udiv_fix_sat; else - IID = ResultFixedSema.isSigned() ? llvm::Intrinsic::sdiv_fix + IID = CommonFixedSema.isSigned() ? llvm::Intrinsic::sdiv_fix : llvm::Intrinsic::udiv_fix; Result = Builder.CreateIntrinsic(IID, {FullLHS->getType()}, {FullLHS, FullRHS, Builder.getInt32(CommonFixedSema.getScale())}); diff --git a/clang/test/Frontend/fixed_point_add.c b/clang/test/Frontend/fixed_point_add.c index be3d5a8f5e9e..078976503ecd 100644 --- a/clang/test/Frontend/fixed_point_add.c +++ b/clang/test/Frontend/fixed_point_add.c @@ -413,7 +413,7 @@ void SaturatedAddition() { // SIGNED-NEXT: [[USA_SAT_RESIZE:%[a-z0-9]+]] = zext i16 [[USA_SAT]] to i40 // SIGNED-NEXT: [[I_RESIZE:%[a-z0-9]+]] = sext i32 [[I]] to i40 // SIGNED-NEXT: [[I_UPSCALE:%[a-z0-9]+]] = shl i40 [[I_RESIZE]], 8 - // SIGNED-NEXT: [[SUM:%[0-9]+]] = call i40 @llvm.uadd.sat.i40(i40 [[USA_SAT_RESIZE]], i40 [[I_UPSCALE]]) + // SIGNED-NEXT: [[SUM:%[0-9]+]] = call i40 @llvm.sadd.sat.i40(i40 [[USA_SAT_RESIZE]], i40 [[I_UPSCALE]]) // SIGNED-NEXT: [[USE_MAX:%[0-9]+]] = icmp sgt i40 [[SUM]], 65535 // SIGNED-NEXT: [[RESULT:%[a-z0-9]+]] = select i1 [[USE_MAX]], i40 65535, i40 [[SUM]] // SIGNED-NEXT:
[clang] 33bae9c - [AST] Fix handling of some edge cases in fixed-point division.
Author: Bevin Hansson Date: 2020-06-30T13:47:12+02:00 New Revision: 33bae9c265486cd37e0612711a863f0a4b865a26 URL: https://github.com/llvm/llvm-project/commit/33bae9c265486cd37e0612711a863f0a4b865a26 DIFF: https://github.com/llvm/llvm-project/commit/33bae9c265486cd37e0612711a863f0a4b865a26.diff LOG: [AST] Fix handling of some edge cases in fixed-point division. Division by zero was not being handled, and division of -EPSILON / MAX did not perform rounding correctly. Added: Modified: clang/lib/AST/ExprConstant.cpp clang/lib/Basic/FixedPoint.cpp clang/test/Frontend/fixed_point_div.c clang/test/Frontend/fixed_point_errors.c Removed: diff --git a/clang/lib/AST/ExprConstant.cpp b/clang/lib/AST/ExprConstant.cpp index ebe09c99429f..9eba40c44ddc 100644 --- a/clang/lib/AST/ExprConstant.cpp +++ b/clang/lib/AST/ExprConstant.cpp @@ -12954,6 +12954,10 @@ bool FixedPointExprEvaluator::VisitBinaryOperator(const BinaryOperator *E) { break; } case BO_Div: { +if (RHSFX.getValue() == 0) { + Info.FFDiag(E, diag::note_expr_divide_by_zero); + return false; +} Result = LHSFX.div(RHSFX, &OpOverflow) .convert(ResultFXSema, &ConversionOverflow); break; diff --git a/clang/lib/Basic/FixedPoint.cpp b/clang/lib/Basic/FixedPoint.cpp index e8db43c6fe90..ed8b92c98fdb 100644 --- a/clang/lib/Basic/FixedPoint.cpp +++ b/clang/lib/Basic/FixedPoint.cpp @@ -282,7 +282,7 @@ APFixedPoint APFixedPoint::div(const APFixedPoint &Other, llvm::APInt::sdivrem(ThisVal, OtherVal, Result, Rem); // If the quotient is negative and the remainder is nonzero, round // towards negative infinity by subtracting epsilon from the result. -if (Result.isNegative() && !Rem.isNullValue()) +if (ThisVal.isNegative() != OtherVal.isNegative() && !Rem.isNullValue()) Result = Result - 1; } else Result = ThisVal.udiv(OtherVal); diff --git a/clang/test/Frontend/fixed_point_div.c b/clang/test/Frontend/fixed_point_div.c index ce1e02387de1..b18ba59ae735 100644 --- a/clang/test/Frontend/fixed_point_div.c +++ b/clang/test/Frontend/fixed_point_div.c @@ -64,6 +64,8 @@ short _Accum sa_const13 = 0.0234375hk / 2.0hk; // CHECK-DAG: @sa_const13 = {{.*}}global i16 1, align 2 short _Accum sa_const14 = -0.0234375hk / 2.0hk; // CHECK-DAG: @sa_const14 = {{.*}}global i16 -2, align 2 +short _Accum sa_const15 = -0.0078125hk / 255.28125hk; +// CHECK-DAG: @sa_const15 = {{.*}}global i16 -1, align 2 void SignedDivision() { // CHECK-LABEL: SignedDivision diff --git a/clang/test/Frontend/fixed_point_errors.c b/clang/test/Frontend/fixed_point_errors.c index c1d80d5daf46..2fd8cfd1fdf7 100644 --- a/clang/test/Frontend/fixed_point_errors.c +++ b/clang/test/Frontend/fixed_point_errors.c @@ -264,3 +264,6 @@ short _Fract add_sat = (_Sat short _Fract)0.5hr + 0.5hr; short _Accum sub_sat = (_Sat short _Accum)-200.0hk - 80.0hk; short _Accum mul_sat = (_Sat short _Accum)80.0hk * 10.0hk; short _Fract div_sat = (_Sat short _Fract)0.9hr / 0.1hr; + +// Division by zero +short _Accum div_zero = 4.5k / 0.0lr; // expected-error {{initializer element is not a compile-time constant}} ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] fd7a341 - [Fixed Point] Move the compassign LHS type correction a bit further down. NFCI.
Author: Bevin Hansson Date: 2020-04-17T10:09:02+02:00 New Revision: fd7a34186137168064ffe2ca536823559b92d939 URL: https://github.com/llvm/llvm-project/commit/fd7a34186137168064ffe2ca536823559b92d939 DIFF: https://github.com/llvm/llvm-project/commit/fd7a34186137168064ffe2ca536823559b92d939.diff LOG: [Fixed Point] Move the compassign LHS type correction a bit further down. NFCI. Summary: We can simplify the LHSTy correction for fixed-point compassign by moving it below the point where we know we have a compound assignment. Also, we shouldn't look at the LHS and RHS separately; look at the computation result type instead. Looking at the LHS and RHS is also wrong for compassigns with fixed and floating point (though this does not work upstream yet). Reviewers: leonardchan Subscribers: cfe-commits Tags: #clang Differential Revision: https://reviews.llvm.org/D78294 Added: Modified: clang/lib/Sema/SemaExpr.cpp Removed: diff --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp index 60d99db7ced9..31d694857e9c 100644 --- a/clang/lib/Sema/SemaExpr.cpp +++ b/clang/lib/Sema/SemaExpr.cpp @@ -13639,14 +13639,6 @@ ExprResult Sema::CreateBuiltinBinOp(SourceLocation OpLoc, if (ResultTy.isNull() || LHS.isInvalid() || RHS.isInvalid()) return ExprError(); - // The LHS is not converted to the result type for fixed-point compound - // assignment as the common type is computed on demand. Reset the CompLHSTy - // to the LHS type we would have gotten after unary conversions. - if (!CompLHSTy.isNull() && - (LHS.get()->getType()->isFixedPointType() || - RHS.get()->getType()->isFixedPointType())) -CompLHSTy = UsualUnaryConversions(LHS.get()).get()->getType(); - if (ResultTy->isRealFloatingType() && (getLangOpts().getFPRoundingMode() != RoundingMode::NearestTiesToEven || getLangOpts().getFPExceptionMode() != LangOptions::FPE_Ignore)) @@ -13705,6 +13697,12 @@ ExprResult Sema::CreateBuiltinBinOp(SourceLocation OpLoc, OK = LHS.get()->getObjectKind(); } + // The LHS is not converted to the result type for fixed-point compound + // assignment as the common type is computed on demand. Reset the CompLHSTy + // to the LHS type we would have gotten after unary conversions. + if (CompResultTy->isFixedPointType()) +CompLHSTy = UsualUnaryConversions(LHS.get()).get()->getType(); + if (ConvertHalfVec) return convertHalfVecBinOp(*this, LHS, RHS, Opc, ResultTy, VK, OK, true, OpLoc, CurFPFeatures); ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] aa0d19a - [Fixed Point] Add fixed-point shift operations and consteval.
Author: Bevin Hansson Date: 2020-08-07T15:09:24+02:00 New Revision: aa0d19a0c8f5ebccb5768411dfc4feddff7bed08 URL: https://github.com/llvm/llvm-project/commit/aa0d19a0c8f5ebccb5768411dfc4feddff7bed08 DIFF: https://github.com/llvm/llvm-project/commit/aa0d19a0c8f5ebccb5768411dfc4feddff7bed08.diff LOG: [Fixed Point] Add fixed-point shift operations and consteval. Reviewers: rjmccall, leonardchan, bjope Subscribers: cfe-commits Tags: #clang Differential Revision: https://reviews.llvm.org/D83212 Added: clang/test/Frontend/fixed_point_shift.c Modified: clang/include/clang/Basic/FixedPoint.h clang/lib/AST/ExprConstant.cpp clang/lib/Basic/FixedPoint.cpp clang/lib/Sema/SemaExpr.cpp clang/test/Frontend/fixed_point_errors.c Removed: diff --git a/clang/include/clang/Basic/FixedPoint.h b/clang/include/clang/Basic/FixedPoint.h index 0d181f30907f..ee52a2b0a615 100644 --- a/clang/include/clang/Basic/FixedPoint.h +++ b/clang/include/clang/Basic/FixedPoint.h @@ -132,17 +132,20 @@ class APFixedPoint { APFixedPoint mul(const APFixedPoint &Other, bool *Overflow = nullptr) const; APFixedPoint div(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 { + // Perform shift operations on a fixed point type. Unlike the other binary + // operations, the resulting fixed point value will be in the original + // semantic. + APFixedPoint shl(unsigned Amt, bool *Overflow = nullptr) const; + APFixedPoint shr(unsigned Amt, bool *Overflow = nullptr) const { +// Right shift cannot overflow. +if (Overflow) + *Overflow = false; return APFixedPoint(Val >> Amt, Sema); } - APFixedPoint shl(unsigned Amt) const { -return APFixedPoint(Val << Amt, Sema); - } + /// Perform a unary negation (-X) on this fixed point type, taking into + /// account saturation if applicable. + APFixedPoint negate(bool *Overflow = nullptr) const; /// Return the integral part of this fixed point number, rounded towards /// zero. (-2.5k -> -2) diff --git a/clang/lib/AST/ExprConstant.cpp b/clang/lib/AST/ExprConstant.cpp index d0587cb723bc..11ba8db24355 100644 --- a/clang/lib/AST/ExprConstant.cpp +++ b/clang/lib/AST/ExprConstant.cpp @@ -13003,6 +13003,29 @@ bool FixedPointExprEvaluator::VisitBinaryOperator(const BinaryOperator *E) { .convert(ResultFXSema, &ConversionOverflow); break; } + case BO_Shl: + case BO_Shr: { +FixedPointSemantics LHSSema = LHSFX.getSemantics(); +llvm::APSInt RHSVal = RHSFX.getValue(); + +unsigned ShiftBW = +LHSSema.getWidth() - (unsigned)LHSSema.hasUnsignedPadding(); +unsigned Amt = RHSVal.getLimitedValue(ShiftBW - 1); +// Embedded-C 4.1.6.2.2: +// The right operand must be nonnegative and less than the total number +// of (nonpadding) bits of the fixed-point operand ... +if (RHSVal.isNegative()) + Info.CCEDiag(E, diag::note_constexpr_negative_shift) << RHSVal; +else if (Amt != RHSVal) + Info.CCEDiag(E, diag::note_constexpr_large_shift) + << RHSVal << E->getType() << ShiftBW; + +if (E->getOpcode() == BO_Shl) + Result = LHSFX.shl(Amt, &OpOverflow); +else + Result = LHSFX.shr(Amt, &OpOverflow); +break; + } default: return false; } diff --git a/clang/lib/Basic/FixedPoint.cpp b/clang/lib/Basic/FixedPoint.cpp index ed8b92c98fdb..892b8c19fd79 100644 --- a/clang/lib/Basic/FixedPoint.cpp +++ b/clang/lib/Basic/FixedPoint.cpp @@ -309,6 +309,40 @@ APFixedPoint APFixedPoint::div(const APFixedPoint &Other, CommonFXSema); } +APFixedPoint APFixedPoint::shl(unsigned Amt, bool *Overflow) const { + llvm::APSInt ThisVal = Val; + bool Overflowed = false; + + // Widen the LHS. + unsigned Wide = Sema.getWidth() * 2; + if (Sema.isSigned()) +ThisVal = ThisVal.sextOrSelf(Wide); + else +ThisVal = ThisVal.zextOrSelf(Wide); + + // Clamp the shift amount at the original width, and perform the shift. + Amt = std::min(Amt, ThisVal.getBitWidth()); + llvm::APSInt Result = ThisVal << Amt; + Result.setIsSigned(Sema.isSigned()); + + // If our result lies outside of the representative range of the + // semantic, we either have overflow or saturation. + llvm::APSInt Max = APFixedPoint::getMax(Sema).getValue().extOrTrunc(Wide); + llvm::APSInt Min = APFixedPoint::getMin(Sema).getValue().extOrTrunc(Wide); + if (Sema.isSaturated()) { +if (Result < Min) + Result = Min; +else if (Result > Max) + Result = Max; + } else +Overflowed = Result < Min || Result > Max; + + if (Overflow) +*Overflow = Overflowed; + + return APFixedPoint(Result.sextOrTrunc(Sema.getWidth()), Sema); +} + void APFixedPoint:
[clang] 956582a - [Sema] Iteratively strip sugar when removing address spaces.
Author: Bevin Hansson Date: 2020-08-11T17:26:19+02:00 New Revision: 956582aa165804dd8335879c3a7f833901e5424c URL: https://github.com/llvm/llvm-project/commit/956582aa165804dd8335879c3a7f833901e5424c DIFF: https://github.com/llvm/llvm-project/commit/956582aa165804dd8335879c3a7f833901e5424c.diff LOG: [Sema] Iteratively strip sugar when removing address spaces. ASTContext::removeAddrSpaceQualType does not properly deal with sugar. QualTypes derive their ASes from the AS on the canonical type, not the type itself. However, removeAddrSpaceQualType only strips the outermost qualifiers, which means that it can fail to remove addrspace qualifiers if there is sugar in the way. Change the function to desugar types until the address space really no longer exists on the corresponding QualType. This should guarantee the removal of the address space. This fixes the erroneous behavior in D62574. Reviewed By: rjmccall, svenvh Differential Revision: https://reviews.llvm.org/D83325 Added: Modified: clang/lib/AST/ASTContext.cpp clang/test/CodeGenCXX/address-space-cast.cpp Removed: diff --git a/clang/lib/AST/ASTContext.cpp b/clang/lib/AST/ASTContext.cpp index 7947de3a31fc..4d708d57cabf 100644 --- a/clang/lib/AST/ASTContext.cpp +++ b/clang/lib/AST/ASTContext.cpp @@ -2934,14 +2934,27 @@ QualType ASTContext::getAddrSpaceQualType(QualType T, } QualType ASTContext::removeAddrSpaceQualType(QualType T) const { + // If the type is not qualified with an address space, just return it + // immediately. + if (!T.hasAddressSpace()) +return T; + // If we are composing extended qualifiers together, merge together // into one ExtQuals node. QualifierCollector Quals; - const Type *TypeNode = Quals.strip(T); + const Type *TypeNode; - // If the qualifier doesn't have an address space just return it. - if (!Quals.hasAddressSpace()) -return T; + while (T.hasAddressSpace()) { +TypeNode = Quals.strip(T); + +// If the type no longer has an address space after stripping qualifiers, +// jump out. +if (!QualType(TypeNode, 0).hasAddressSpace()) + break; + +// There might be sugar in the way. Strip it and try again. +T = T.getSingleStepDesugaredType(*this); + } Quals.removeAddressSpace(); diff --git a/clang/test/CodeGenCXX/address-space-cast.cpp b/clang/test/CodeGenCXX/address-space-cast.cpp index e0cf3c199b88..7b0792df8588 100644 --- a/clang/test/CodeGenCXX/address-space-cast.cpp +++ b/clang/test/CodeGenCXX/address-space-cast.cpp @@ -6,6 +6,16 @@ void func_pchar(__private__ char *x); void func_pvoid(__private__ void *x); void func_pint(__private__ int *x); +class Base { +}; + +class Derived : public Base { +}; + +void fn(Derived *p) { + __private__ Base *b = (__private__ Base *)p; +} + void test_cast(char *gen_char_ptr, void *gen_void_ptr, int *gen_int_ptr) { // CHECK: %[[cast:.*]] = addrspacecast i8* %{{.*}} to i8 addrspace(5)* // CHECK-NEXT: store i8 addrspace(5)* %[[cast]] ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] d4408fe - [clang] Do not crash for unsupported fixed point to floating point conversion
Author: Gousemoodhin Nadaf Date: 2020-08-11T17:26:19+02:00 New Revision: d4408fe17f33bcd664ec8f468abfd1094e84a7c1 URL: https://github.com/llvm/llvm-project/commit/d4408fe17f33bcd664ec8f468abfd1094e84a7c1 DIFF: https://github.com/llvm/llvm-project/commit/d4408fe17f33bcd664ec8f468abfd1094e84a7c1.diff LOG: [clang] Do not crash for unsupported fixed point to floating point conversion - Fixed point to floating point conversion is unimplemented. - If one of the operands has a floating type and the other operand has a fixed-point type, the function handleFloatConversion() is called because one of the operands has a floating type, but we do not handle fixed point type in this function (Implementation of fixed point to floating point conversion is missing), due to this compiler crashes. In order to avoid compiler crash, when one of the operands has a floating type and the other operand has a fixed-point type, return NULL. - FIXME: Implementation of fixed point to floating point conversion. - I am going to resolve FIXME in followup patches. - Add the test case. Reviewed By: ebevhan Differential Revision: https://reviews.llvm.org/D81904 Added: Modified: clang/lib/Sema/SemaExpr.cpp clang/test/Frontend/fixed_point_errors.c Removed: diff --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp index 79340a9c7d7b..bea693f999ab 100644 --- a/clang/lib/Sema/SemaExpr.cpp +++ b/clang/lib/Sema/SemaExpr.cpp @@ -1125,6 +1125,11 @@ static QualType handleFloatConversion(Sema &S, ExprResult &LHS, bool LHSFloat = LHSType->isRealFloatingType(); bool RHSFloat = RHSType->isRealFloatingType(); + // FIXME: Implement floating to fixed point conversion.(Bug 46268) + // Reference N1169 4.1.4 (Type conversion, usual arithmetic conversions). + if ((LHSType->isFixedPointType() && RHSFloat) || + (LHSFloat && RHSType->isFixedPointType())) +return QualType(); // If we have two real floating types, convert the smaller operand // to the bigger result. if (LHSFloat && RHSFloat) { diff --git a/clang/test/Frontend/fixed_point_errors.c b/clang/test/Frontend/fixed_point_errors.c index 6c41bf6df163..5aaf59876dcb 100644 --- a/clang/test/Frontend/fixed_point_errors.c +++ b/clang/test/Frontend/fixed_point_errors.c @@ -286,3 +286,8 @@ short _Accum shl_sat = (_Sat short _Accum)200.0hk << 5; // Division by zero short _Accum div_zero = 4.5k / 0.0lr; // expected-error {{initializer element is not a compile-time constant}} + +void foo(void) { + _Accum x = 0.5k; + if (x == 0.5) {} // expected-error{{invalid operands to binary expression ('_Accum' and 'double')}} +} ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang][C++26] Implement Pack Indexing (P2662R3). (PR #72644)
bevin-hansson wrote: Ping. Just wondering if anyone has seen the above. @cor3ntin ? https://github.com/llvm/llvm-project/pull/72644 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[llvm] [pstl] [clang] [mlir] [libcxx] [clang-tools-extra] [lld] [openmp] [lldb] [ELF] Don't resolve relocations referencing SHN_ABS to tombstone in non-SHF_ALLOC sections (PR #79238)
https://github.com/bevin-hansson commented: This looks good to me, but someone else should probably chime in as well to approve. https://github.com/llvm/llvm-project/pull/79238 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang][C++26] Implement Pack Indexing (P2662R3). (PR #72644)
https://github.com/bevin-hansson commented: We hit some odd behavior in our downstream after this patch. https://github.com/llvm/llvm-project/pull/72644 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang][C++26] Implement Pack Indexing (P2662R3). (PR #72644)
https://github.com/bevin-hansson edited https://github.com/llvm/llvm-project/pull/72644 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang][C++26] Implement Pack Indexing (P2662R3). (PR #72644)
@@ -4934,6 +4934,73 @@ class DependentDecltypeType : public DecltypeType, public llvm::FoldingSetNode { Expr *E); }; +class PackIndexingType final +: public Type, + public llvm::FoldingSetNode, + private llvm::TrailingObjects { + friend TrailingObjects; + + const ASTContext &Context; + QualType Pattern; + Expr *IndexExpr; + + unsigned Size; + +protected: + friend class ASTContext; // ASTContext creates these. + PackIndexingType(const ASTContext &Context, QualType Canonical, + QualType Pattern, Expr *IndexExpr, + ArrayRef Expansions = {}); + +public: + Expr *getIndexExpr() const { return IndexExpr; } + QualType getPattern() const { return Pattern; } + + bool isSugared() const { return hasSelectedType(); } + + QualType desugar() const { +if (hasSelectedType()) + return getSelectedType(); +return QualType(this, 0); + } bevin-hansson wrote: Is this correct? Calling `getTypeInfo` on a PackIndexingType where hasSelectedType is false will cause it to infinitely recurse as it tries to desugar itself over and over again. This seems to happen if the type is broken, like the not_pack examples in cxx2c-pack-indexing.cpp. Maybe the issue lies elsewhere, but something is strange with the error recovery for these types. Perhaps because `isDependentType` isn't true for them when they are broken? https://github.com/llvm/llvm-project/pull/72644 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang][C++26] Implement Pack Indexing (P2662R3). (PR #72644)
@@ -4934,6 +4934,73 @@ class DependentDecltypeType : public DecltypeType, public llvm::FoldingSetNode { Expr *E); }; +class PackIndexingType final +: public Type, + public llvm::FoldingSetNode, + private llvm::TrailingObjects { + friend TrailingObjects; + + const ASTContext &Context; + QualType Pattern; + Expr *IndexExpr; + + unsigned Size; + +protected: + friend class ASTContext; // ASTContext creates these. + PackIndexingType(const ASTContext &Context, QualType Canonical, + QualType Pattern, Expr *IndexExpr, + ArrayRef Expansions = {}); + +public: + Expr *getIndexExpr() const { return IndexExpr; } + QualType getPattern() const { return Pattern; } + + bool isSugared() const { return hasSelectedType(); } + + QualType desugar() const { +if (hasSelectedType()) + return getSelectedType(); +return QualType(this, 0); + } + + QualType getSelectedType() const { +assert(hasSelectedType() && "Type is dependant"); bevin-hansson wrote: Trying to dump/print a broken pack indexing expression/type breaks on this assertion. Try adding -ast-dump to cxx2c-pack-indexing.cpp and see what happens. https://github.com/llvm/llvm-project/pull/72644 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] 1e7ec48 - [AST] Get field size in chars rather than bits in RecordLayoutBuilder.
Author: Bevin Hansson Date: 2020-08-20T10:29:29+02:00 New Revision: 1e7ec4842c1a8b0c686e5674e215012867938a8d URL: https://github.com/llvm/llvm-project/commit/1e7ec4842c1a8b0c686e5674e215012867938a8d DIFF: https://github.com/llvm/llvm-project/commit/1e7ec4842c1a8b0c686e5674e215012867938a8d.diff LOG: [AST] Get field size in chars rather than bits in RecordLayoutBuilder. In D79719, LayoutField was refactored to fetch the size of field types in bits and then convert to chars, rather than fetching them in chars directly. This is not ideal, since it makes the calculations char size dependent, and breaks for sizes that are not a multiple of the char size. This patch changes it to use getTypeInfoInChars instead of getTypeInfo. Reviewed By: efriedma Differential Revision: https://reviews.llvm.org/D85191 Added: Modified: clang/lib/AST/RecordLayoutBuilder.cpp Removed: diff --git a/clang/lib/AST/RecordLayoutBuilder.cpp b/clang/lib/AST/RecordLayoutBuilder.cpp index 0afe91b446ee..715b629e290d 100644 --- a/clang/lib/AST/RecordLayoutBuilder.cpp +++ b/clang/lib/AST/RecordLayoutBuilder.cpp @@ -1838,14 +1838,13 @@ void ItaniumRecordLayoutBuilder::LayoutField(const FieldDecl *D, CharUnits EffectiveFieldSize; auto setDeclInfo = [&](bool IsIncompleteArrayType) { -TypeInfo TI = Context.getTypeInfo(D->getType()); -FieldAlign = Context.toCharUnitsFromBits(TI.Align); +auto TI = Context.getTypeInfoInChars(D->getType()); +FieldAlign = TI.second; // Flexible array members don't have any size, but they have to be // aligned appropriately for their element type. EffectiveFieldSize = FieldSize = -IsIncompleteArrayType ? CharUnits::Zero() - : Context.toCharUnitsFromBits(TI.Width); -AlignIsRequired = TI.AlignIsRequired; +IsIncompleteArrayType ? CharUnits::Zero() : TI.first; +AlignIsRequired = Context.getTypeInfo(D->getType()).AlignIsRequired; }; if (D->getType()->isIncompleteArrayType()) { ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] 1a995a0 - [ADT] Move FixedPoint.h from Clang to LLVM.
Author: Bevin Hansson Date: 2020-08-20T10:29:45+02:00 New Revision: 1a995a0af3c4e97c1135b6c9c3a243072ee0463c URL: https://github.com/llvm/llvm-project/commit/1a995a0af3c4e97c1135b6c9c3a243072ee0463c DIFF: https://github.com/llvm/llvm-project/commit/1a995a0af3c4e97c1135b6c9c3a243072ee0463c.diff LOG: [ADT] Move FixedPoint.h from Clang to LLVM. This patch moves FixedPointSemantics and APFixedPoint from Clang to LLVM ADT. This will make it easier to use the fixed-point classes in LLVM for constructing an IR builder for fixed-point and for reusing the APFixedPoint class for constant evaluation purposes. RFC: http://lists.llvm.org/pipermail/llvm-dev/2020-August/144025.html Reviewed By: leonardchan, rjmccall Differential Revision: https://reviews.llvm.org/D85312 Added: llvm/include/llvm/ADT/APFixedPoint.h llvm/lib/Support/APFixedPoint.cpp llvm/unittests/ADT/APFixedPointTest.cpp Modified: clang/include/clang/AST/APValue.h clang/include/clang/AST/ASTContext.h clang/include/clang/AST/Expr.h clang/include/clang/AST/OptionalDiagnostic.h clang/lib/AST/ASTContext.cpp clang/lib/AST/ExprConstant.cpp clang/lib/AST/Type.cpp clang/lib/Basic/CMakeLists.txt clang/lib/CodeGen/CGExprScalar.cpp clang/lib/Sema/SemaChecking.cpp clang/lib/Sema/SemaExpr.cpp clang/lib/Serialization/ASTReader.cpp clang/lib/Serialization/ASTWriter.cpp clang/unittests/Basic/CMakeLists.txt llvm/lib/Support/CMakeLists.txt llvm/unittests/ADT/CMakeLists.txt Removed: clang/include/clang/Basic/FixedPoint.h clang/lib/Basic/FixedPoint.cpp clang/unittests/Basic/FixedPointTest.cpp diff --git a/clang/include/clang/AST/APValue.h b/clang/include/clang/AST/APValue.h index cca92b5f8235..87e4bd7f84c1 100644 --- a/clang/include/clang/AST/APValue.h +++ b/clang/include/clang/AST/APValue.h @@ -13,8 +13,8 @@ #ifndef LLVM_CLANG_AST_APVALUE_H #define LLVM_CLANG_AST_APVALUE_H -#include "clang/Basic/FixedPoint.h" #include "clang/Basic/LLVM.h" +#include "llvm/ADT/APFixedPoint.h" #include "llvm/ADT/APFloat.h" #include "llvm/ADT/APSInt.h" #include "llvm/ADT/PointerIntPair.h" @@ -32,6 +32,7 @@ namespace clang { struct PrintingPolicy; class Type; class ValueDecl; + class QualType; /// Symbolic representation of typeid(T) for some type T. class TypeInfoLValue { @@ -113,6 +114,7 @@ namespace clang { /// [APSInt] [APFloat], [Complex APSInt] [Complex APFloat], [Expr + Offset], /// [Vector: N * APValue], [Array: N * APValue] class APValue { + typedef llvm::APFixedPoint APFixedPoint; typedef llvm::APSInt APSInt; typedef llvm::APFloat APFloat; public: diff --git a/clang/include/clang/AST/ASTContext.h b/clang/include/clang/AST/ASTContext.h index 3996644c9c85..b71f144d22a3 100644 --- a/clang/include/clang/AST/ASTContext.h +++ b/clang/include/clang/AST/ASTContext.h @@ -74,6 +74,8 @@ namespace llvm { +class APFixedPoint; +class FixedPointSemantics; struct fltSemantics; template class SmallPtrSet; @@ -81,7 +83,6 @@ template class SmallPtrSet; namespace clang { -class APFixedPoint; class APValue; class ASTMutationListener; class ASTRecordLayout; @@ -99,7 +100,6 @@ class ParentMapContext; class DynTypedNode; class DynTypedNodeList; class Expr; -class FixedPointSemantics; class GlobalDecl; class MangleContext; class MangleNumberingContext; @@ -1985,9 +1985,9 @@ class ASTContext : public RefCountedBase { unsigned char getFixedPointScale(QualType Ty) const; unsigned char getFixedPointIBits(QualType Ty) const; - FixedPointSemantics getFixedPointSemantics(QualType Ty) const; - APFixedPoint getFixedPointMax(QualType Ty) const; - APFixedPoint getFixedPointMin(QualType Ty) const; + llvm::FixedPointSemantics getFixedPointSemantics(QualType Ty) const; + llvm::APFixedPoint getFixedPointMax(QualType Ty) const; + llvm::APFixedPoint getFixedPointMin(QualType Ty) const; DeclarationNameInfo getNameForTemplate(TemplateName Name, SourceLocation NameLoc) const; diff --git a/clang/include/clang/AST/Expr.h b/clang/include/clang/AST/Expr.h index 96db7bc3be29..5edca2593789 100644 --- a/clang/include/clang/AST/Expr.h +++ b/clang/include/clang/AST/Expr.h @@ -24,7 +24,6 @@ #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" diff --git a/clang/include/clang/AST/OptionalDiagnostic.h b/clang/include/clang/AST/OptionalDiagnostic.h index c57199f0fdf1..c9a2d19f4ebc 100644 --- a/clang/include/clang/AST/OptionalDiagnostic.h +++ b/clang/include/clang/AST/OptionalDiagnostic.h @@ -63,7 +63,7 @@ class OptionalDiagnostic { return *this; } - OptionalDiagnostic &operator<<(const APFixedPoint &FX) { + OptionalDiagnostic &op
[clang] 2bac004 - Add triples to fixed-point tests which lacked them.
Author: Bevin Hansson Date: 2020-08-20T15:36:15+02:00 New Revision: 2bac004c905dc8db99fd3766678d33aa5a0eec2b URL: https://github.com/llvm/llvm-project/commit/2bac004c905dc8db99fd3766678d33aa5a0eec2b DIFF: https://github.com/llvm/llvm-project/commit/2bac004c905dc8db99fd3766678d33aa5a0eec2b.diff LOG: Add triples to fixed-point tests which lacked them. This caused failures on clang-x390x-linux. Added: Modified: clang/test/Frontend/fixed_point_mul.c clang/test/Frontend/fixed_point_sub.c clang/test/Frontend/fixed_point_sub_const.c Removed: diff --git a/clang/test/Frontend/fixed_point_mul.c b/clang/test/Frontend/fixed_point_mul.c index 05d3f77d5a124..777c35c52d4a3 100644 --- a/clang/test/Frontend/fixed_point_mul.c +++ b/clang/test/Frontend/fixed_point_mul.c @@ -1,6 +1,6 @@ // NOTE: Assertions have been autogenerated by utils/update_cc_test_checks.py -// 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 short _Accum sa; _Accum a, a2, a3, a4; diff --git a/clang/test/Frontend/fixed_point_sub.c b/clang/test/Frontend/fixed_point_sub.c index 4e97f5d1ec589..4d07b4a522572 100644 --- a/clang/test/Frontend/fixed_point_sub.c +++ b/clang/test/Frontend/fixed_point_sub.c @@ -1,6 +1,6 @@ // NOTE: Assertions have been autogenerated by utils/update_cc_test_checks.py -// 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 short _Accum sa; _Accum a, a2, a3, a4; diff --git a/clang/test/Frontend/fixed_point_sub_const.c b/clang/test/Frontend/fixed_point_sub_const.c index 219ce5a411c1d..dc6ad92ec798d 100644 --- a/clang/test/Frontend/fixed_point_sub_const.c +++ b/clang/test/Frontend/fixed_point_sub_const.c @@ -1,5 +1,5 @@ -// 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 // Subtraction between diff erent fixed point types short _Accum sa_const = 1.0hk - 2.0hk; ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] 577f8b1 - [Fixed Point] Add codegen for fixed-point shifts.
Author: Bevin Hansson Date: 2020-08-24T14:37:16+02:00 New Revision: 577f8b157a03055821341146ed0617e3b103fdaf URL: https://github.com/llvm/llvm-project/commit/577f8b157a03055821341146ed0617e3b103fdaf DIFF: https://github.com/llvm/llvm-project/commit/577f8b157a03055821341146ed0617e3b103fdaf.diff LOG: [Fixed Point] Add codegen for fixed-point shifts. This patch adds codegen to Clang for fixed-point shift operations. Reviewed By: leonardchan Differential Revision: https://reviews.llvm.org/D83294 Added: clang/test/Frontend/fixed_point_shift_const.c Modified: clang/lib/CodeGen/CGExprScalar.cpp clang/test/Frontend/fixed_point_compound.c clang/test/Frontend/fixed_point_shift.c Removed: diff --git a/clang/lib/CodeGen/CGExprScalar.cpp b/clang/lib/CodeGen/CGExprScalar.cpp index b3857b3eeb06..d0ec50f4e011 100644 --- a/clang/lib/CodeGen/CGExprScalar.cpp +++ b/clang/lib/CodeGen/CGExprScalar.cpp @@ -3535,6 +3535,14 @@ Value *ScalarExprEmitter::EmitFixedPointBinOp(const BinOpInfo &op) { case BO_Div: Result = FPBuilder.CreateDiv(LHS, LHSFixedSema, RHS, RHSFixedSema); break; + case BO_ShlAssign: + case BO_Shl: +Result = FPBuilder.CreateShl(LHS, LHSFixedSema, RHS); +break; + case BO_ShrAssign: + case BO_Shr: +Result = FPBuilder.CreateShr(LHS, LHSFixedSema, RHS); +break; case BO_LT: return FPBuilder.CreateLT(LHS, LHSFixedSema, RHS, RHSFixedSema); case BO_GT: @@ -3550,13 +3558,9 @@ Value *ScalarExprEmitter::EmitFixedPointBinOp(const BinOpInfo &op) { return FPBuilder.CreateEQ(LHS, LHSFixedSema, RHS, RHSFixedSema); case BO_NE: return FPBuilder.CreateNE(LHS, LHSFixedSema, RHS, RHSFixedSema); - case BO_Shl: - case BO_Shr: case BO_Cmp: case BO_LAnd: case BO_LOr: - case BO_ShlAssign: - case BO_ShrAssign: llvm_unreachable("Found unimplemented fixed point binary operation"); case BO_PtrMemD: case BO_PtrMemI: @@ -3573,8 +3577,12 @@ Value *ScalarExprEmitter::EmitFixedPointBinOp(const BinOpInfo &op) { llvm_unreachable("Found unsupported binary operation for fixed point types."); } + bool IsShift = BinaryOperator::isShiftOp(op.Opcode) || + BinaryOperator::isShiftAssignOp(op.Opcode); // Convert to the result type. - return FPBuilder.CreateFixedToFixed(Result, CommonFixedSema, ResultFixedSema); + return FPBuilder.CreateFixedToFixed(Result, IsShift ? LHSFixedSema + : CommonFixedSema, + ResultFixedSema); } Value *ScalarExprEmitter::EmitSub(const BinOpInfo &op) { @@ -3701,6 +3709,10 @@ Value *ScalarExprEmitter::ConstrainShiftValue(Value *LHS, Value *RHS, } Value *ScalarExprEmitter::EmitShl(const BinOpInfo &Ops) { + // TODO: This misses out on the sanitizer check below. + if (Ops.isFixedPointOp()) +return EmitFixedPointBinOp(Ops); + // LLVM requires the LHS and RHS to be the same type: promote or truncate the // RHS to the same size as the LHS. Value *RHS = Ops.RHS; @@ -3768,6 +3780,10 @@ Value *ScalarExprEmitter::EmitShl(const BinOpInfo &Ops) { } Value *ScalarExprEmitter::EmitShr(const BinOpInfo &Ops) { + // TODO: This misses out on the sanitizer check below. + if (Ops.isFixedPointOp()) +return EmitFixedPointBinOp(Ops); + // LLVM requires the LHS and RHS to be the same type: promote or truncate the // RHS to the same size as the LHS. Value *RHS = Ops.RHS; diff --git a/clang/test/Frontend/fixed_point_compound.c b/clang/test/Frontend/fixed_point_compound.c index 4a44d0ae95a2..897ba2e22636 100644 --- a/clang/test/Frontend/fixed_point_compound.c +++ b/clang/test/Frontend/fixed_point_compound.c @@ -567,3 +567,50 @@ void div_csa() { c /= sa; } + +// CHECK-LABEL: @shft_ai( +// CHECK-NEXT: entry: +// CHECK-NEXT:[[TMP0:%.*]] = load i32, i32* @i, align 4 +// CHECK-NEXT:[[TMP1:%.*]] = load i32, i32* @a, align 4 +// CHECK-NEXT:[[TMP2:%.*]] = shl i32 [[TMP1]], [[TMP0]] +// CHECK-NEXT:store i32 [[TMP2]], i32* @a, align 4 +// CHECK-NEXT:ret void +// +void shft_ai() { + a <<= i; +} + +// SIGNED-LABEL: @shft_sufi( +// SIGNED-NEXT: entry: +// SIGNED-NEXT:[[TMP0:%.*]] = load i32, i32* @i, align 4 +// SIGNED-NEXT:[[TMP1:%.*]] = load i16, i16* @suf, align 2 +// SIGNED-NEXT:[[TMP2:%.*]] = trunc i32 [[TMP0]] to i16 +// SIGNED-NEXT:[[TMP3:%.*]] = call i16 @llvm.ushl.sat.i16(i16 [[TMP1]], i16 [[TMP2]]) +// SIGNED-NEXT:store i16 [[TMP3]], i16* @suf, align 2 +// SIGNED-NEXT:ret void +// +// UNSIGNED-LABEL: @shft_sufi( +// UNSIGNED-NEXT: entry: +// UNSIGNED-NEXT:[[TMP0:%.*]] = load i32, i32* @i, align 4 +// UNSIGNED-NEXT:[[TMP1:%.*]] = load i16, i16* @suf, align 2 +// UNSIGNED-NEXT:[[TMP2:%.*]] = trunc i32 [[TMP0]] to i16 +// UNSIGNED-NEXT:[[TMP3:%.*]] = call i16 @llvm.sshl.sat.i16(i16 [[TMP1]], i16 [[TMP2]]) +// UNSIGNED-NEXT:st
[clang] 808ac54 - [Fixed Point] Use FixedPointBuilder to codegen fixed-point IR.
Author: Bevin Hansson Date: 2020-08-24T14:37:07+02:00 New Revision: 808ac54645212ddc9aba150cdc97454e36fb9521 URL: https://github.com/llvm/llvm-project/commit/808ac54645212ddc9aba150cdc97454e36fb9521 DIFF: https://github.com/llvm/llvm-project/commit/808ac54645212ddc9aba150cdc97454e36fb9521.diff LOG: [Fixed Point] Use FixedPointBuilder to codegen fixed-point IR. This changes the methods in CGExprScalar to use FixedPointBuilder to generate IR for fixed-point conversions and operations. Since FixedPointBuilder emits padded operations slightly differently than the original code, some tests change. Reviewed By: leonardchan Differential Revision: https://reviews.llvm.org/D86282 Added: Modified: clang/lib/CodeGen/CGExprScalar.cpp clang/test/Frontend/fixed_point_add.c clang/test/Frontend/fixed_point_div.c clang/test/Frontend/fixed_point_mul.c clang/test/Frontend/fixed_point_sub.c clang/test/Frontend/fixed_point_unary.c Removed: diff --git a/clang/lib/CodeGen/CGExprScalar.cpp b/clang/lib/CodeGen/CGExprScalar.cpp index aad4d2d2a674..b3857b3eeb06 100644 --- a/clang/lib/CodeGen/CGExprScalar.cpp +++ b/clang/lib/CodeGen/CGExprScalar.cpp @@ -32,6 +32,7 @@ #include "llvm/IR/CFG.h" #include "llvm/IR/Constants.h" #include "llvm/IR/DataLayout.h" +#include "llvm/IR/FixedPointBuilder.h" #include "llvm/IR/Function.h" #include "llvm/IR/GetElementPtrTypeIterator.h" #include "llvm/IR/GlobalVariable.h" @@ -356,11 +357,6 @@ class ScalarExprEmitter /// and an integer. Value *EmitFixedPointConversion(Value *Src, QualType SrcTy, QualType DstTy, SourceLocation Loc); - Value *EmitFixedPointConversion(Value *Src, - llvm::FixedPointSemantics &SrcFixedSema, - llvm::FixedPointSemantics &DstFixedSema, - SourceLocation Loc, - bool DstIsInteger = false); /// Emit a conversion from the specified complex type to the specified /// destination type, where the destination type is an LLVM scalar type. @@ -1447,91 +1443,17 @@ Value *ScalarExprEmitter::EmitFixedPointConversion(Value *Src, QualType SrcTy, SourceLocation Loc) { auto SrcFPSema = CGF.getContext().getFixedPointSemantics(SrcTy); auto DstFPSema = CGF.getContext().getFixedPointSemantics(DstTy); - return EmitFixedPointConversion(Src, SrcFPSema, DstFPSema, Loc, - DstTy->isIntegerType()); -} - -Value *ScalarExprEmitter::EmitFixedPointConversion( -Value *Src, llvm::FixedPointSemantics &SrcFPSema, -llvm::FixedPointSemantics &DstFPSema, -SourceLocation Loc, bool DstIsInteger) { - using llvm::APFixedPoint; - using llvm::APInt; - using llvm::ConstantInt; - using llvm::Value; - - unsigned SrcWidth = SrcFPSema.getWidth(); - unsigned DstWidth = DstFPSema.getWidth(); - unsigned SrcScale = SrcFPSema.getScale(); - unsigned DstScale = DstFPSema.getScale(); - bool SrcIsSigned = SrcFPSema.isSigned(); - bool DstIsSigned = DstFPSema.isSigned(); - - llvm::Type *DstIntTy = Builder.getIntNTy(DstWidth); - - Value *Result = Src; - unsigned ResultWidth = SrcWidth; - - // Downscale. - if (DstScale < SrcScale) { -// When converting to integers, we round towards zero. For negative numbers, -// right shifting rounds towards negative infinity. In this case, we can -// just round up before shifting. -if (DstIsInteger && SrcIsSigned) { - Value *Zero = llvm::Constant::getNullValue(Result->getType()); - Value *IsNegative = Builder.CreateICmpSLT(Result, Zero); - Value *LowBits = ConstantInt::get( - CGF.getLLVMContext(), APInt::getLowBitsSet(ResultWidth, SrcScale)); - Value *Rounded = Builder.CreateAdd(Result, LowBits); - Result = Builder.CreateSelect(IsNegative, Rounded, Result); -} - -Result = SrcIsSigned - ? Builder.CreateAShr(Result, SrcScale - DstScale, "downscale") - : Builder.CreateLShr(Result, SrcScale - DstScale, "downscale"); - } - - if (!DstFPSema.isSaturated()) { -// Resize. -Result = Builder.CreateIntCast(Result, DstIntTy, SrcIsSigned, "resize"); - -// Upscale. -if (DstScale > SrcScale) - Result = Builder.CreateShl(Result, DstScale - SrcScale, "upscale"); - } else { -// Adjust the number of fractional bits. -if (DstScale > SrcScale) { - // Compare to DstWidth to prevent resizing twice. - ResultWidth = std::max(SrcWidth + DstScale - SrcScale, DstWidth); - llvm::Type *UpscaledTy = Builder.getIntNTy(ResultWidth); - Result = Builder.CreateIntCast(Result, UpscaledTy, SrcIsSigned, "resize"); - Result = Builder.CreateShl(Result, DstScale - SrcScale, "upscale"); -} - -// Handle saturation. -bool LessIntBits = DstFPSema.getIntegralBi
[clang] 9c26eb8 - Refactor fixed point conversion test.
Author: Bevin Hansson Date: 2020-10-09T10:27:42+02:00 New Revision: 9c26eb8b915e5e20afa7d3e07996ea112c18ccc3 URL: https://github.com/llvm/llvm-project/commit/9c26eb8b915e5e20afa7d3e07996ea112c18ccc3 DIFF: https://github.com/llvm/llvm-project/commit/9c26eb8b915e5e20afa7d3e07996ea112c18ccc3.diff LOG: Refactor fixed point conversion test. Differential Revision: https://reviews.llvm.org/D88648 Added: clang/test/Frontend/fixed_point_conversions_const.c Modified: clang/test/Frontend/fixed_point_conversions.c Removed: diff --git a/clang/test/Frontend/fixed_point_conversions.c b/clang/test/Frontend/fixed_point_conversions.c index 86a687bdef93..dfe727c708f4 100644 --- a/clang/test/Frontend/fixed_point_conversions.c +++ b/clang/test/Frontend/fixed_point_conversions.c @@ -1,485 +1,697 @@ +// NOTE: Assertions have been autogenerated by utils/update_cc_test_checks.py // 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 -S -emit-llvm %s -o - -fpadding-on-unsigned-fixed-point | FileCheck %s --check-prefixes=CHECK,UNSIGNED -// Between diff erent fixed point types -short _Accum sa_const = 2.5hk; // CHECK-DAG: @sa_const = {{.*}}global i16 320, align 2 -_Accum a_const = 2.5hk;// CHECK-DAG: @a_const = {{.*}}global i32 81920, align 4 -short _Accum sa_const2 = 2.5k; // CHECK-DAG: @sa_const2 = {{.*}}global i16 320, align 2 - -short _Accum sa_from_f_const = 0.5r; // CHECK-DAG: sa_from_f_const = {{.*}}global i16 64, align 2 -_Fract f_from_sa_const = 0.5hk; // CHECK-DAG: f_from_sa_const = {{.*}}global i16 16384, align 2 - -unsigned short _Accum usa_const = 2.5uk; -unsigned _Accum ua_const = 2.5uhk; -// SIGNED-DAG: @usa_const = {{.*}}global i16 640, align 2 -// SIGNED-DAG: @ua_const = {{.*}}global i32 163840, align 4 -// UNSIGNED-DAG:@usa_const = {{.*}}global i16 320, align 2 -// UNSIGNED-DAG:@ua_const = {{.*}}global i32 81920, align 4 - -// FixedPoint to integer -int i_const = -128.0hk; // CHECK-DAG: @i_const = {{.*}}global i32 -128, align 4 -int i_const2 = 128.0hk; // CHECK-DAG: @i_const2 = {{.*}}global i32 128, align 4 -int i_const3 = -128.0k; // CHECK-DAG: @i_const3 = {{.*}}global i32 -128, align 4 -int i_const4 = 128.0k; // CHECK-DAG: @i_const4 = {{.*}}global i32 128, align 4 -short s_const = -128.0k; // CHECK-DAG: @s_const = {{.*}}global i16 -128, align 2 -short s_const2 = 128.0k; // CHECK-DAG: @s_const2 = {{.*}}global i16 128, align 2 - -// Integer to fixed point -short _Accum sa_const5 = 2;// CHECK-DAG: @sa_const5 = {{.*}}global i16 256, align 2 -short _Accum sa_const6 = -2; // CHECK-DAG: @sa_const6 = {{.*}}global i16 -256, align 2 -short _Accum sa_const7 = -256; // CHECK-DAG: @sa_const7 = {{.*}}global i16 -32768, align 2 - -// Signedness -unsigned short _Accum usa_const2 = 2.5hk; -// SIGNED-DAG: @usa_const2 = {{.*}}global i16 640, align 2 -// UNSIGNED-DAG:@usa_const2 = {{.*}}global i16 320, align 2 -short _Accum sa_const3 = 2.5hk; // CHECK-DAG: @sa_const3 = {{.*}}global i16 320, align 2 - -int i_const5 = 128.0uhk; -unsigned int ui_const = 128.0hk; -// CHECK-DAG: @i_const5 = {{.*}}global i32 128, align 4 -// CHECK-DAG: @ui_const = {{.*}}global i32 128, align 4 - -short _Accum sa_const9 = 2u; // CHECK-DAG: @sa_const9 = {{.*}}global i16 256, align 2 -unsigned short _Accum usa_const3 = 2; -// SIGNED-DAG: @usa_const3 = {{.*}}global i16 512, align 2 -// UNSIGNED-DAG:@usa_const3 = {{.*}}global i16 256, align 2 - -// Overflow (this is undefined but allowed) -short _Accum sa_const4 = 256.0k; -unsigned int ui_const2 = -2.5hk; -short _Accum sa_const8 = 256; -unsigned short _Accum usa_const4 = -2; - -// Saturation -_Sat short _Accum sat_sa_const = 2.5hk; // CHECK-DAG: @sat_sa_const = {{.*}}global i16 320, align 2 -_Sat short _Accum sat_sa_const2 = 256.0k; // CHECK-DAG: @sat_sa_const2 = {{.*}}global i16 32767, align 2 -_Sat unsigned short _Accum sat_usa_const = -1.0hk; -// CHECK-DAG: @sat_usa_const = {{.*}}global i16 0, align 2 -_Sat unsigned short _Accum sat_usa_const2 = 256.0k; -// SIGNED-DAG: @sat_usa_const2 = {{.*}}global i16 -1, align 2 -// UNSIGNED-DAG:@sat_usa_const2 = {{.*}}global i16 32767, align 2 - -_Sat short _Accum sat_sa_const3 = 256; // CHECK-DAG: @sat_sa_const3 = {{.*}}global i16 32767, align 2 -_Sat short _Accum sat_sa_const4 = -257; // CHECK-DAG: @sat_sa_const4 = {{.*}}global i16 -32768, align 2 -_Sat unsigned short _Accum sat_usa_const3 = -1; -// CHECK-DAG: @sat_usa_const3 = {{.*}}global i16 0, align 2 -_Sat unsigned short _Accum sat_usa_const4 = 256; -// SIGNED-DAG: @sat_usa_const4 = {{.*}}global i16 -1, align 2 -// UNSIGNED-DAG:@sat_usa_const4 = {{.*}}global i16 32767, align 2 - -void TestFixedPointCastSameType() { - _Accum a = 2.5k; - _Accum a2 = a; - // CHECK: [
[clang] 9fa7f48 - [Fixed Point] Add fixed-point to floating point cast types and consteval.
Author: Bevin Hansson Date: 2020-10-13T13:26:56+02:00 New Revision: 9fa7f48459761fa13205f4c931484b0977c35746 URL: https://github.com/llvm/llvm-project/commit/9fa7f48459761fa13205f4c931484b0977c35746 DIFF: https://github.com/llvm/llvm-project/commit/9fa7f48459761fa13205f4c931484b0977c35746.diff LOG: [Fixed Point] Add fixed-point to floating point cast types and consteval. Reviewed By: leonardchan Differential Revision: https://reviews.llvm.org/D86631 Added: Modified: clang/include/clang/AST/OperationKinds.def clang/lib/AST/Expr.cpp clang/lib/AST/ExprConstant.cpp clang/lib/CodeGen/CGExpr.cpp clang/lib/CodeGen/CGExprAgg.cpp clang/lib/CodeGen/CGExprComplex.cpp clang/lib/CodeGen/CGExprConstant.cpp clang/lib/CodeGen/CGExprScalar.cpp clang/lib/Edit/RewriteObjCFoundationAPI.cpp clang/lib/Sema/SemaExpr.cpp clang/lib/StaticAnalyzer/Core/ExprEngineC.cpp clang/test/Frontend/fixed_point_conversions_const.c clang/test/Frontend/fixed_point_errors.c clang/test/Frontend/fixed_point_unknown_conversions.c Removed: diff --git a/clang/include/clang/AST/OperationKinds.def b/clang/include/clang/AST/OperationKinds.def index f29664e8eb33..6daab1ffcb0a 100644 --- a/clang/include/clang/AST/OperationKinds.def +++ b/clang/include/clang/AST/OperationKinds.def @@ -201,6 +201,14 @@ CAST_OPERATION(IntegralToBoolean) ///float f = i; CAST_OPERATION(IntegralToFloating) +/// CK_FloatingToFixedPoint - Floating to fixed point. +///_Accum a = f; +CAST_OPERATION(FloatingToFixedPoint) + +/// CK_FixedPointToFloating - Fixed point to floating. +///(float) 2.5k +CAST_OPERATION(FixedPointToFloating) + /// CK_FixedPointCast - Fixed point to fixed point. ///(_Accum) 0.5r CAST_OPERATION(FixedPointCast) diff --git a/clang/lib/AST/Expr.cpp b/clang/lib/AST/Expr.cpp index 8e8dd75e975a..919d3220875c 100644 --- a/clang/lib/AST/Expr.cpp +++ b/clang/lib/AST/Expr.cpp @@ -1691,6 +1691,8 @@ bool CastExpr::CastConsistency() const { case CK_ARCExtendBlockObject: case CK_ZeroToOCLOpaqueType: case CK_IntToOCLSampler: + case CK_FloatingToFixedPoint: + case CK_FixedPointToFloating: case CK_FixedPointCast: case CK_FixedPointToIntegral: case CK_IntegralToFixedPoint: diff --git a/clang/lib/AST/ExprConstant.cpp b/clang/lib/AST/ExprConstant.cpp index 639a5733b34b..1327aa6876e4 100644 --- a/clang/lib/AST/ExprConstant.cpp +++ b/clang/lib/AST/ExprConstant.cpp @@ -12896,6 +12896,8 @@ bool IntExprEvaluator::VisitCastExpr(const CastExpr *E) { case CK_NonAtomicToAtomic: case CK_AddressSpaceConversion: case CK_IntToOCLSampler: + case CK_FloatingToFixedPoint: + case CK_FixedPointToFloating: case CK_FixedPointCast: case CK_IntegralToFixedPoint: llvm_unreachable("invalid cast kind for integral value"); @@ -13140,6 +13142,26 @@ bool FixedPointExprEvaluator::VisitCastExpr(const CastExpr *E) { return Success(IntResult, E); } + case CK_FloatingToFixedPoint: { +APFloat Src(0.0); +if (!EvaluateFloat(SubExpr, Src, Info)) + return false; + +bool Overflowed; +APFixedPoint Result = APFixedPoint::getFromFloatValue( +Src, Info.Ctx.getFixedPointSemantics(DestType), &Overflowed); + +if (Overflowed) { + if (Info.checkingForUndefinedBehavior()) +Info.Ctx.getDiagnostics().Report(E->getExprLoc(), + diag::warn_fixedpoint_constant_overflow) + << Result.toString() << E->getType(); + else if (!HandleOverflow(Info, E, Result, E->getType())) +return false; +} + +return Success(Result, E); + } case CK_NoOp: case CK_LValueToRValue: return ExprEvaluatorBaseTy::VisitCastExpr(E); @@ -13446,6 +13468,15 @@ bool FloatExprEvaluator::VisitCastExpr(const CastExpr *E) { E->getType(), Result); } + case CK_FixedPointToFloating: { +APFixedPoint FixResult(Info.Ctx.getFixedPointSemantics(SubExpr->getType())); +if (!EvaluateFixedPoint(SubExpr, FixResult, Info)) + return false; +Result = +FixResult.convertToFloat(Info.Ctx.getFloatTypeSemantics(E->getType())); +return true; + } + case CK_FloatingCast: { if (!Visit(SubExpr)) return false; @@ -13591,6 +13622,8 @@ bool ComplexExprEvaluator::VisitCastExpr(const CastExpr *E) { case CK_NonAtomicToAtomic: case CK_AddressSpaceConversion: case CK_IntToOCLSampler: + case CK_FloatingToFixedPoint: + case CK_FixedPointToFloating: case CK_FixedPointCast: case CK_FixedPointToBoolean: case CK_FixedPointToIntegral: diff --git a/clang/lib/CodeGen/CGExpr.cpp b/clang/lib/CodeGen/CGExpr.cpp index 869bace18ffc..2f54097d9209 100644 --- a/clang/lib/CodeGen/CGExpr.cpp +++ b/clang/lib/CodeGen/CGExpr.cpp @@ -4593,6 +4593,8 @@ LValue CodeGenFunction::EmitCastLValue(const CastExpr *E) { case CK_ARCExtendBlockObject: case CK_Co
[clang] 101309f - [AST] Change return type of getTypeInfoInChars to a proper struct instead of std::pair.
Author: Bevin Hansson Date: 2020-10-13T13:26:56+02:00 New Revision: 101309fe048e66873cfd972c47c4b7e7f2b99f41 URL: https://github.com/llvm/llvm-project/commit/101309fe048e66873cfd972c47c4b7e7f2b99f41 DIFF: https://github.com/llvm/llvm-project/commit/101309fe048e66873cfd972c47c4b7e7f2b99f41.diff LOG: [AST] Change return type of getTypeInfoInChars to a proper struct instead of std::pair. Followup to D85191. This changes getTypeInfoInChars to return a TypeInfoChars struct instead of a std::pair of CharUnits. This lets the interface match getTypeInfo more closely. Reviewed By: efriedma Differential Revision: https://reviews.llvm.org/D86447 Added: Modified: clang/include/clang/AST/ASTContext.h clang/lib/AST/ASTContext.cpp clang/lib/AST/RecordLayoutBuilder.cpp clang/lib/CodeGen/CGAtomic.cpp clang/lib/CodeGen/CGBlocks.cpp clang/lib/CodeGen/CGCUDANV.cpp clang/lib/CodeGen/CGCall.cpp clang/lib/CodeGen/CGClass.cpp clang/lib/CodeGen/CGExprAgg.cpp clang/lib/CodeGen/CGObjC.cpp clang/lib/CodeGen/CGRecordLayoutBuilder.cpp clang/lib/CodeGen/CGValue.h clang/lib/CodeGen/TargetInfo.cpp clang/lib/StaticAnalyzer/Checkers/PaddingChecker.cpp Removed: diff --git a/clang/include/clang/AST/ASTContext.h b/clang/include/clang/AST/ASTContext.h index e261e29036e9..3f4079e2569b 100644 --- a/clang/include/clang/AST/ASTContext.h +++ b/clang/include/clang/AST/ASTContext.h @@ -171,6 +171,16 @@ struct TypeInfo { : Width(Width), Align(Align), AlignIsRequired(AlignIsRequired) {} }; +struct TypeInfoChars { + CharUnits Width; + CharUnits Align; + bool AlignIsRequired : 1; + + TypeInfoChars() : AlignIsRequired(false) {} + TypeInfoChars(CharUnits Width, CharUnits Align, bool AlignIsRequired) + : Width(Width), Align(Align), AlignIsRequired(AlignIsRequired) {} +}; + /// Holds long-lived AST nodes (such as types and decls) that can be /// referred to throughout the semantic analysis of a file. class ASTContext : public RefCountedBase { @@ -2169,10 +2179,10 @@ class ASTContext : public RefCountedBase { // getTypeInfoDataSizeInChars - Return the size of a type, in chars. If the // type is a record, its data size is returned. - std::pair getTypeInfoDataSizeInChars(QualType T) const; + TypeInfoChars getTypeInfoDataSizeInChars(QualType T) const; - std::pair getTypeInfoInChars(const Type *T) const; - std::pair getTypeInfoInChars(QualType T) const; + TypeInfoChars getTypeInfoInChars(const Type *T) const; + TypeInfoChars getTypeInfoInChars(QualType T) const; /// Determine if the alignment the type has was required using an /// alignment attribute. diff --git a/clang/lib/AST/ASTContext.cpp b/clang/lib/AST/ASTContext.cpp index a82d95461bb9..8caff6b33379 100644 --- a/clang/lib/AST/ASTContext.cpp +++ b/clang/lib/AST/ASTContext.cpp @@ -1777,9 +1777,8 @@ CharUnits ASTContext::getExnObjectAlignment() const { // chars. If the type is a record, its data size is returned. This is // the size of the memcpy that's performed when assigning this type // using a trivial copy/move assignment operator. -std::pair -ASTContext::getTypeInfoDataSizeInChars(QualType T) const { - std::pair sizeAndAlign = getTypeInfoInChars(T); +TypeInfoChars ASTContext::getTypeInfoDataSizeInChars(QualType T) const { + TypeInfoChars Info = getTypeInfoInChars(T); // In C++, objects can sometimes be allocated into the tail padding // of a base-class subobject. We decide whether that's possible @@ -1787,44 +1786,43 @@ ASTContext::getTypeInfoDataSizeInChars(QualType T) const { if (getLangOpts().CPlusPlus) { if (const auto *RT = T->getAs()) { const ASTRecordLayout &layout = getASTRecordLayout(RT->getDecl()); - sizeAndAlign.first = layout.getDataSize(); + Info.Width = layout.getDataSize(); } } - return sizeAndAlign; + return Info; } /// getConstantArrayInfoInChars - Performing the computation in CharUnits /// instead of in bits prevents overflowing the uint64_t for some large arrays. -std::pair +TypeInfoChars static getConstantArrayInfoInChars(const ASTContext &Context, const ConstantArrayType *CAT) { - std::pair EltInfo = - Context.getTypeInfoInChars(CAT->getElementType()); + TypeInfoChars EltInfo = Context.getTypeInfoInChars(CAT->getElementType()); uint64_t Size = CAT->getSize().getZExtValue(); - assert((Size == 0 || static_cast(EltInfo.first.getQuantity()) <= + assert((Size == 0 || static_cast(EltInfo.Width.getQuantity()) <= (uint64_t)(-1)/Size) && "Overflow in array type char size evaluation"); - uint64_t Width = EltInfo.first.getQuantity() * Size; - unsigned Align = EltInfo.second.getQuantity(); + uint64_t Width = EltInfo.Width.getQuantity() * Size; + unsigned Align = EltInfo.Align.getQuantity(); if (!Context.getTargetInfo().getCXXABI().isMicrosoft() ||
[clang] [clang/AST] Make it possible to use SwiftAttr in type context (PR #108631)
bevin-hansson wrote: Hi @xedin! We've observed a difference downstream due to this patch and are curious whether this was intentional. It seems that the changes to how AttributedType is keyed (including the attribute) causes some type duplication when attributes are involved. For example, building this (reduced) program with `clang -target x86_64 -fsanitize=undefined`: ``` void a() { for (unsigned int b;; *(const unsigned int __attribute__((noderef)) *)*(const unsigned int __attribute__((noderef)) *)b) ; } ``` (Ignore that there are dereferenced pointers with `deref`; the original repro had `address_space` but you don't get sanitizers for such pointers upstream) Before this patch, there would only be a single type-info struct/string in the resulting assembly, but with the patch, there are now two identical ones: ``` .type .L__unnamed_3,@object # @0 .section.rodata,"a",@progbits .p2align4, 0x0 .L__unnamed_3: .short 0 # 0x0 .short 10 # 0xa .asciz "'unsigned int const __attribute__((noderef))'" .size .L__unnamed_3, 50 .type .L__unnamed_1,@object # @1 .data .p2align4, 0x0 .L__unnamed_1: .quad .L.src .long 4 # 0x4 .long 73 # 0x49 .quad .L__unnamed_3 .byte 2 # 0x2 .byte 0 # 0x0 .zero 6 .size .L__unnamed_1, 32 - .type .L__unnamed_2,@object # @2 + .type .L__unnamed_4,@object # @2 + .section.rodata,"a",@progbits + .p2align4, 0x0 +.L__unnamed_4: + .short 0 # 0x0 + .short 10 # 0xa + .asciz "'unsigned int const __attribute__((noderef))'" + .size .L__unnamed_4, 50 + + .type .L__unnamed_2,@object # @3 + .data .p2align4, 0x0 .L__unnamed_2: .quad .L.src .long 4 # 0x4 .long 25 # 0x19 - .quad .L__unnamed_3 + .quad .L__unnamed_4 .byte 2 # 0x2 .byte 0 # 0x0 .zero 6 .size .L__unnamed_2, 32 ``` This is possibly happening for the sanitizer emission due to the code in CodeGenFunction::EmitCheckTypeDescriptor: {code} // Only emit each type's descriptor once. if (llvm::Constant *C = CGM.getTypeDescriptorFromMap(T)) return C; {code} The two types are different for map purposes and type creation (since they have different syntactical Attrs) but the actual types are really the same. I guess this is pretty rare, but it could cause some hefty duplication depending on what types are used and how. There might be other effects I don't know of either, but this was the noticeable one for us. https://github.com/llvm/llvm-project/pull/108631 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang/AST] Make it possible to use SwiftAttr in type context (PR #108631)
bevin-hansson wrote: Well, as far as I know Attrs are created on the spot and aren't uniqued. Perhaps that should happen for these kinds of Attrs. https://github.com/llvm/llvm-project/pull/108631 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clangd] Augment code completion results with documentation from the index. (PR #120099)
https://github.com/bevin-hansson created https://github.com/llvm/llvm-project/pull/120099 When looking up code completions from Sema, there is no associated documentation. This is due to crash issues with stale preambles. However, this also means that code completion results from other than the main file do not have documentation in certain cases, which is a bad user experience. This patch performs a lookup into the index using the code completion result declarations to find documentation, and attaches it to the results. This fixes clangd/clangd#2252 and clangd/clangd#564. >From 16bd4a00feec97ae65a14600caf6921045d54833 Mon Sep 17 00:00:00 2001 From: Bevin Hansson Date: Mon, 16 Dec 2024 16:40:06 +0100 Subject: [PATCH] [clangd] Augment code completion results with documentation from the index. When looking up code completions from Sema, there is no associated documentation. This is due to crash issues with stale preambles. However, this also means that code completion results from other than the main file do not have documentation in certain cases, which is a bad user experience. This patch performs a lookup into the index using the code completion result declarations to find documentation, and attaches it to the results. This fixes clangd/clangd#2252 and clangd/clangd#564. --- clang-tools-extra/clangd/CodeComplete.cpp | 27 +++ 1 file changed, 27 insertions(+) diff --git a/clang-tools-extra/clangd/CodeComplete.cpp b/clang-tools-extra/clangd/CodeComplete.cpp index 2c2d5f0b5ac924..6916a32b7c2bac 100644 --- a/clang-tools-extra/clangd/CodeComplete.cpp +++ b/clang-tools-extra/clangd/CodeComplete.cpp @@ -1863,14 +1863,41 @@ class CodeCompleteFlow { CodeCompleteResult Output; // Convert the results to final form, assembling the expensive strings. +// If necessary, search the index for documentation comments. +LookupRequest Req; +llvm::DenseMap SymbolToCompletion; for (auto &C : Scored) { Output.Completions.push_back(toCodeCompletion(C.first)); Output.Completions.back().Score = C.second; Output.Completions.back().CompletionTokenRange = ReplacedRange; + if (Opts.Index && !Output.Completions.back().Documentation) { +for (auto &Cand : C.first) { + if (Cand.SemaResult && + Cand.SemaResult->Kind == CodeCompletionResult::RK_Declaration) { +auto ID = clangd::getSymbolID(Cand.SemaResult->getDeclaration()); +if (!ID) + continue; +Req.IDs.insert(ID); +SymbolToCompletion[ID] = Output.Completions.size() - 1; + } +} + } } Output.HasMore = Incomplete; Output.Context = CCContextKind; Output.CompletionRange = ReplacedRange; + +// Look up documentation from the index. +if (Opts.Index) { + Opts.Index->lookup(Req, [&](const Symbol &S) { +auto &C = Output.Completions[SymbolToCompletion.at(S.ID)]; +if (S.Documentation.empty()) + return; +C.Documentation.emplace(); +parseDocumentation(S.Documentation, *C.Documentation); + }); +} + return Output; } ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clangd] Augment code completion results with documentation from the index. (PR #120099)
bevin-hansson wrote: I'm not entirely sure how to add to the tests for clangd. Any advice? https://github.com/llvm/llvm-project/pull/120099 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clangd] Augment code completion results with documentation from the index. (PR #120099)
https://github.com/bevin-hansson updated https://github.com/llvm/llvm-project/pull/120099 >From 16bd4a00feec97ae65a14600caf6921045d54833 Mon Sep 17 00:00:00 2001 From: Bevin Hansson Date: Mon, 16 Dec 2024 16:40:06 +0100 Subject: [PATCH 1/2] [clangd] Augment code completion results with documentation from the index. When looking up code completions from Sema, there is no associated documentation. This is due to crash issues with stale preambles. However, this also means that code completion results from other than the main file do not have documentation in certain cases, which is a bad user experience. This patch performs a lookup into the index using the code completion result declarations to find documentation, and attaches it to the results. This fixes clangd/clangd#2252 and clangd/clangd#564. --- clang-tools-extra/clangd/CodeComplete.cpp | 27 +++ 1 file changed, 27 insertions(+) diff --git a/clang-tools-extra/clangd/CodeComplete.cpp b/clang-tools-extra/clangd/CodeComplete.cpp index 2c2d5f0b5ac924..6916a32b7c2bac 100644 --- a/clang-tools-extra/clangd/CodeComplete.cpp +++ b/clang-tools-extra/clangd/CodeComplete.cpp @@ -1863,14 +1863,41 @@ class CodeCompleteFlow { CodeCompleteResult Output; // Convert the results to final form, assembling the expensive strings. +// If necessary, search the index for documentation comments. +LookupRequest Req; +llvm::DenseMap SymbolToCompletion; for (auto &C : Scored) { Output.Completions.push_back(toCodeCompletion(C.first)); Output.Completions.back().Score = C.second; Output.Completions.back().CompletionTokenRange = ReplacedRange; + if (Opts.Index && !Output.Completions.back().Documentation) { +for (auto &Cand : C.first) { + if (Cand.SemaResult && + Cand.SemaResult->Kind == CodeCompletionResult::RK_Declaration) { +auto ID = clangd::getSymbolID(Cand.SemaResult->getDeclaration()); +if (!ID) + continue; +Req.IDs.insert(ID); +SymbolToCompletion[ID] = Output.Completions.size() - 1; + } +} + } } Output.HasMore = Incomplete; Output.Context = CCContextKind; Output.CompletionRange = ReplacedRange; + +// Look up documentation from the index. +if (Opts.Index) { + Opts.Index->lookup(Req, [&](const Symbol &S) { +auto &C = Output.Completions[SymbolToCompletion.at(S.ID)]; +if (S.Documentation.empty()) + return; +C.Documentation.emplace(); +parseDocumentation(S.Documentation, *C.Documentation); + }); +} + return Output; } >From ace8121506029e9016df9ec3bffa9d465334e97d Mon Sep 17 00:00:00 2001 From: Bevin Hansson Date: Tue, 17 Dec 2024 08:57:52 +0100 Subject: [PATCH 2/2] Add test. --- .../clangd/unittests/CodeCompleteTests.cpp| 37 +++ 1 file changed, 37 insertions(+) diff --git a/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp b/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp index 3acacf496e77f9..827b64c882d583 100644 --- a/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp +++ b/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp @@ -1101,6 +1101,43 @@ int x = foo^ Contains(AllOf(named("foo"), doc("This comment should be retained!"; } +TEST(CompletionTest, CommentsOnMembers) { + MockFS FS; + MockCompilationDatabase CDB; + + auto Opts = ClangdServer::optsForTest(); + Opts.BuildDynamicSymbolIndex = true; + + ClangdServer Server(CDB, FS, Opts); + + FS.Files[testPath("foo.h")] = R"cpp( +struct alpha { + /// This is a member field. + int gamma; + + /// This is a member function. + int delta(); +}; + )cpp"; + + auto File = testPath("foo.cpp"); + Annotations Test(R"cpp( +#include "foo.h" +alpha a; +int x = a.^ + )cpp"); + runAddDocument(Server, File, Test.code()); + auto CompletionList = + llvm::cantFail(runCodeComplete(Server, File, Test.point(), {})); + + EXPECT_THAT( + CompletionList.Completions, + Contains(AllOf(named("gamma"), doc("This is a member field."; + EXPECT_THAT( + CompletionList.Completions, + Contains(AllOf(named("delta"), doc("This is a member function."; +} + TEST(CompletionTest, GlobalCompletionFiltering) { Symbol Class = cls("XYZ"); ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clangd] Augment code completion results with documentation from the index. (PR #120099)
bevin-hansson wrote: > > I'm not entirely sure how to add to the tests for clangd. Any advice? > > We have GTest unit tests for code completion in `CodeCompleteTests.cpp`. > [This > test](https://searchfox.org/llvm/rev/2df48fa78b496a2d276aa848598634bb2aad6857/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp#1074-1102) > is a good starting point -- the test cases we want would be pretty similar, > except we want to test class members (and we don't care whether it's in a > system header specifically). Thanks for the pointer! I added a test based on that one. https://github.com/llvm/llvm-project/pull/120099 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clangd] Augment code completion results with documentation from the index. (PR #120099)
https://github.com/bevin-hansson updated https://github.com/llvm/llvm-project/pull/120099 >From 16bd4a00feec97ae65a14600caf6921045d54833 Mon Sep 17 00:00:00 2001 From: Bevin Hansson Date: Mon, 16 Dec 2024 16:40:06 +0100 Subject: [PATCH 1/3] [clangd] Augment code completion results with documentation from the index. When looking up code completions from Sema, there is no associated documentation. This is due to crash issues with stale preambles. However, this also means that code completion results from other than the main file do not have documentation in certain cases, which is a bad user experience. This patch performs a lookup into the index using the code completion result declarations to find documentation, and attaches it to the results. This fixes clangd/clangd#2252 and clangd/clangd#564. --- clang-tools-extra/clangd/CodeComplete.cpp | 27 +++ 1 file changed, 27 insertions(+) diff --git a/clang-tools-extra/clangd/CodeComplete.cpp b/clang-tools-extra/clangd/CodeComplete.cpp index 2c2d5f0b5ac924..6916a32b7c2bac 100644 --- a/clang-tools-extra/clangd/CodeComplete.cpp +++ b/clang-tools-extra/clangd/CodeComplete.cpp @@ -1863,14 +1863,41 @@ class CodeCompleteFlow { CodeCompleteResult Output; // Convert the results to final form, assembling the expensive strings. +// If necessary, search the index for documentation comments. +LookupRequest Req; +llvm::DenseMap SymbolToCompletion; for (auto &C : Scored) { Output.Completions.push_back(toCodeCompletion(C.first)); Output.Completions.back().Score = C.second; Output.Completions.back().CompletionTokenRange = ReplacedRange; + if (Opts.Index && !Output.Completions.back().Documentation) { +for (auto &Cand : C.first) { + if (Cand.SemaResult && + Cand.SemaResult->Kind == CodeCompletionResult::RK_Declaration) { +auto ID = clangd::getSymbolID(Cand.SemaResult->getDeclaration()); +if (!ID) + continue; +Req.IDs.insert(ID); +SymbolToCompletion[ID] = Output.Completions.size() - 1; + } +} + } } Output.HasMore = Incomplete; Output.Context = CCContextKind; Output.CompletionRange = ReplacedRange; + +// Look up documentation from the index. +if (Opts.Index) { + Opts.Index->lookup(Req, [&](const Symbol &S) { +auto &C = Output.Completions[SymbolToCompletion.at(S.ID)]; +if (S.Documentation.empty()) + return; +C.Documentation.emplace(); +parseDocumentation(S.Documentation, *C.Documentation); + }); +} + return Output; } >From ace8121506029e9016df9ec3bffa9d465334e97d Mon Sep 17 00:00:00 2001 From: Bevin Hansson Date: Tue, 17 Dec 2024 08:57:52 +0100 Subject: [PATCH 2/3] Add test. --- .../clangd/unittests/CodeCompleteTests.cpp| 37 +++ 1 file changed, 37 insertions(+) diff --git a/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp b/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp index 3acacf496e77f9..827b64c882d583 100644 --- a/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp +++ b/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp @@ -1101,6 +1101,43 @@ int x = foo^ Contains(AllOf(named("foo"), doc("This comment should be retained!"; } +TEST(CompletionTest, CommentsOnMembers) { + MockFS FS; + MockCompilationDatabase CDB; + + auto Opts = ClangdServer::optsForTest(); + Opts.BuildDynamicSymbolIndex = true; + + ClangdServer Server(CDB, FS, Opts); + + FS.Files[testPath("foo.h")] = R"cpp( +struct alpha { + /// This is a member field. + int gamma; + + /// This is a member function. + int delta(); +}; + )cpp"; + + auto File = testPath("foo.cpp"); + Annotations Test(R"cpp( +#include "foo.h" +alpha a; +int x = a.^ + )cpp"); + runAddDocument(Server, File, Test.code()); + auto CompletionList = + llvm::cantFail(runCodeComplete(Server, File, Test.point(), {})); + + EXPECT_THAT( + CompletionList.Completions, + Contains(AllOf(named("gamma"), doc("This is a member field."; + EXPECT_THAT( + CompletionList.Completions, + Contains(AllOf(named("delta"), doc("This is a member function."; +} + TEST(CompletionTest, GlobalCompletionFiltering) { Symbol Class = cls("XYZ"); >From 53978eccd6689486088aa5b76f39758357e339c4 Mon Sep 17 00:00:00 2001 From: Bevin Hansson Date: Tue, 17 Dec 2024 09:33:03 +0100 Subject: [PATCH 3/3] Make clang-format happy. --- clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp b/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp index 827b64c882d583..607236c5b07925 100644 --- a/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp +++ b/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp @@ -1130,9 +11
[clang-tools-extra] [clangd] Augment code completion results with documentation from the index. (PR #120099)
bevin-hansson wrote: I've come across an issue with this, again. This seems to work for headers I've written myself (or at least ones in my own project), but for e.g. standard library include types, it doesn't work. Something like ``` std::string a; a.^ ``` still contains no documentation in the autocompletion results even though there are doccomments in the header in question. Maybe this is related to how the standard library headers are indexed somehow... https://github.com/llvm/llvm-project/pull/120099 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clangd] Augment code completion results with documentation from the index. (PR #120099)
https://github.com/bevin-hansson updated https://github.com/llvm/llvm-project/pull/120099 >From 16bd4a00feec97ae65a14600caf6921045d54833 Mon Sep 17 00:00:00 2001 From: Bevin Hansson Date: Mon, 16 Dec 2024 16:40:06 +0100 Subject: [PATCH 1/4] [clangd] Augment code completion results with documentation from the index. When looking up code completions from Sema, there is no associated documentation. This is due to crash issues with stale preambles. However, this also means that code completion results from other than the main file do not have documentation in certain cases, which is a bad user experience. This patch performs a lookup into the index using the code completion result declarations to find documentation, and attaches it to the results. This fixes clangd/clangd#2252 and clangd/clangd#564. --- clang-tools-extra/clangd/CodeComplete.cpp | 27 +++ 1 file changed, 27 insertions(+) diff --git a/clang-tools-extra/clangd/CodeComplete.cpp b/clang-tools-extra/clangd/CodeComplete.cpp index 2c2d5f0b5ac924..6916a32b7c2bac 100644 --- a/clang-tools-extra/clangd/CodeComplete.cpp +++ b/clang-tools-extra/clangd/CodeComplete.cpp @@ -1863,14 +1863,41 @@ class CodeCompleteFlow { CodeCompleteResult Output; // Convert the results to final form, assembling the expensive strings. +// If necessary, search the index for documentation comments. +LookupRequest Req; +llvm::DenseMap SymbolToCompletion; for (auto &C : Scored) { Output.Completions.push_back(toCodeCompletion(C.first)); Output.Completions.back().Score = C.second; Output.Completions.back().CompletionTokenRange = ReplacedRange; + if (Opts.Index && !Output.Completions.back().Documentation) { +for (auto &Cand : C.first) { + if (Cand.SemaResult && + Cand.SemaResult->Kind == CodeCompletionResult::RK_Declaration) { +auto ID = clangd::getSymbolID(Cand.SemaResult->getDeclaration()); +if (!ID) + continue; +Req.IDs.insert(ID); +SymbolToCompletion[ID] = Output.Completions.size() - 1; + } +} + } } Output.HasMore = Incomplete; Output.Context = CCContextKind; Output.CompletionRange = ReplacedRange; + +// Look up documentation from the index. +if (Opts.Index) { + Opts.Index->lookup(Req, [&](const Symbol &S) { +auto &C = Output.Completions[SymbolToCompletion.at(S.ID)]; +if (S.Documentation.empty()) + return; +C.Documentation.emplace(); +parseDocumentation(S.Documentation, *C.Documentation); + }); +} + return Output; } >From ace8121506029e9016df9ec3bffa9d465334e97d Mon Sep 17 00:00:00 2001 From: Bevin Hansson Date: Tue, 17 Dec 2024 08:57:52 +0100 Subject: [PATCH 2/4] Add test. --- .../clangd/unittests/CodeCompleteTests.cpp| 37 +++ 1 file changed, 37 insertions(+) diff --git a/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp b/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp index 3acacf496e77f9..827b64c882d583 100644 --- a/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp +++ b/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp @@ -1101,6 +1101,43 @@ int x = foo^ Contains(AllOf(named("foo"), doc("This comment should be retained!"; } +TEST(CompletionTest, CommentsOnMembers) { + MockFS FS; + MockCompilationDatabase CDB; + + auto Opts = ClangdServer::optsForTest(); + Opts.BuildDynamicSymbolIndex = true; + + ClangdServer Server(CDB, FS, Opts); + + FS.Files[testPath("foo.h")] = R"cpp( +struct alpha { + /// This is a member field. + int gamma; + + /// This is a member function. + int delta(); +}; + )cpp"; + + auto File = testPath("foo.cpp"); + Annotations Test(R"cpp( +#include "foo.h" +alpha a; +int x = a.^ + )cpp"); + runAddDocument(Server, File, Test.code()); + auto CompletionList = + llvm::cantFail(runCodeComplete(Server, File, Test.point(), {})); + + EXPECT_THAT( + CompletionList.Completions, + Contains(AllOf(named("gamma"), doc("This is a member field."; + EXPECT_THAT( + CompletionList.Completions, + Contains(AllOf(named("delta"), doc("This is a member function."; +} + TEST(CompletionTest, GlobalCompletionFiltering) { Symbol Class = cls("XYZ"); >From 53978eccd6689486088aa5b76f39758357e339c4 Mon Sep 17 00:00:00 2001 From: Bevin Hansson Date: Tue, 17 Dec 2024 09:33:03 +0100 Subject: [PATCH 3/4] Make clang-format happy. --- clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp b/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp index 827b64c882d583..607236c5b07925 100644 --- a/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp +++ b/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp @@ -1130,9 +11
[clang-tools-extra] [clangd] Augment code completion results with documentation from the index. (PR #120099)
https://github.com/bevin-hansson updated https://github.com/llvm/llvm-project/pull/120099 >From 16bd4a00feec97ae65a14600caf6921045d54833 Mon Sep 17 00:00:00 2001 From: Bevin Hansson Date: Mon, 16 Dec 2024 16:40:06 +0100 Subject: [PATCH 1/5] [clangd] Augment code completion results with documentation from the index. When looking up code completions from Sema, there is no associated documentation. This is due to crash issues with stale preambles. However, this also means that code completion results from other than the main file do not have documentation in certain cases, which is a bad user experience. This patch performs a lookup into the index using the code completion result declarations to find documentation, and attaches it to the results. This fixes clangd/clangd#2252 and clangd/clangd#564. --- clang-tools-extra/clangd/CodeComplete.cpp | 27 +++ 1 file changed, 27 insertions(+) diff --git a/clang-tools-extra/clangd/CodeComplete.cpp b/clang-tools-extra/clangd/CodeComplete.cpp index 2c2d5f0b5ac924..6916a32b7c2bac 100644 --- a/clang-tools-extra/clangd/CodeComplete.cpp +++ b/clang-tools-extra/clangd/CodeComplete.cpp @@ -1863,14 +1863,41 @@ class CodeCompleteFlow { CodeCompleteResult Output; // Convert the results to final form, assembling the expensive strings. +// If necessary, search the index for documentation comments. +LookupRequest Req; +llvm::DenseMap SymbolToCompletion; for (auto &C : Scored) { Output.Completions.push_back(toCodeCompletion(C.first)); Output.Completions.back().Score = C.second; Output.Completions.back().CompletionTokenRange = ReplacedRange; + if (Opts.Index && !Output.Completions.back().Documentation) { +for (auto &Cand : C.first) { + if (Cand.SemaResult && + Cand.SemaResult->Kind == CodeCompletionResult::RK_Declaration) { +auto ID = clangd::getSymbolID(Cand.SemaResult->getDeclaration()); +if (!ID) + continue; +Req.IDs.insert(ID); +SymbolToCompletion[ID] = Output.Completions.size() - 1; + } +} + } } Output.HasMore = Incomplete; Output.Context = CCContextKind; Output.CompletionRange = ReplacedRange; + +// Look up documentation from the index. +if (Opts.Index) { + Opts.Index->lookup(Req, [&](const Symbol &S) { +auto &C = Output.Completions[SymbolToCompletion.at(S.ID)]; +if (S.Documentation.empty()) + return; +C.Documentation.emplace(); +parseDocumentation(S.Documentation, *C.Documentation); + }); +} + return Output; } >From ace8121506029e9016df9ec3bffa9d465334e97d Mon Sep 17 00:00:00 2001 From: Bevin Hansson Date: Tue, 17 Dec 2024 08:57:52 +0100 Subject: [PATCH 2/5] Add test. --- .../clangd/unittests/CodeCompleteTests.cpp| 37 +++ 1 file changed, 37 insertions(+) diff --git a/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp b/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp index 3acacf496e77f9..827b64c882d583 100644 --- a/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp +++ b/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp @@ -1101,6 +1101,43 @@ int x = foo^ Contains(AllOf(named("foo"), doc("This comment should be retained!"; } +TEST(CompletionTest, CommentsOnMembers) { + MockFS FS; + MockCompilationDatabase CDB; + + auto Opts = ClangdServer::optsForTest(); + Opts.BuildDynamicSymbolIndex = true; + + ClangdServer Server(CDB, FS, Opts); + + FS.Files[testPath("foo.h")] = R"cpp( +struct alpha { + /// This is a member field. + int gamma; + + /// This is a member function. + int delta(); +}; + )cpp"; + + auto File = testPath("foo.cpp"); + Annotations Test(R"cpp( +#include "foo.h" +alpha a; +int x = a.^ + )cpp"); + runAddDocument(Server, File, Test.code()); + auto CompletionList = + llvm::cantFail(runCodeComplete(Server, File, Test.point(), {})); + + EXPECT_THAT( + CompletionList.Completions, + Contains(AllOf(named("gamma"), doc("This is a member field."; + EXPECT_THAT( + CompletionList.Completions, + Contains(AllOf(named("delta"), doc("This is a member function."; +} + TEST(CompletionTest, GlobalCompletionFiltering) { Symbol Class = cls("XYZ"); >From 53978eccd6689486088aa5b76f39758357e339c4 Mon Sep 17 00:00:00 2001 From: Bevin Hansson Date: Tue, 17 Dec 2024 09:33:03 +0100 Subject: [PATCH 3/5] Make clang-format happy. --- clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp b/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp index 827b64c882d583..607236c5b07925 100644 --- a/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp +++ b/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp @@ -1130,9 +11
[clang-tools-extra] [clangd] Augment code completion results with documentation from the index. (PR #120099)
bevin-hansson wrote: Thanks for merging! https://github.com/llvm/llvm-project/pull/120099 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits