llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-clang-modules @llvm/pr-subscribers-clang Author: cor3ntin (cor3ntin) <details> <summary>Changes</summary> The bulk of the changes are in `CallExpr` We cache Begin/End source locs in the trailing objects, in the space left by making the offset to the trailing objects static. We also set a flag to indicate that we are calling an explicit object member function, further reducing the cost of getBeginLoc. Fixes #<!-- -->140876 --- Patch is 27.20 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/141058.diff 10 Files Affected: - (modified) clang/include/clang/AST/Expr.h (+98-19) - (modified) clang/include/clang/AST/NestedNameSpecifier.h (+11-5) - (modified) clang/include/clang/AST/Stmt.h (+11-7) - (modified) clang/lib/AST/Expr.cpp (+48-76) - (modified) clang/lib/AST/ExprCXX.cpp (+28-14) - (modified) clang/lib/AST/NestedNameSpecifier.cpp (-12) - (modified) clang/lib/Sema/SemaOpenCL.cpp (+2-1) - (modified) clang/lib/Sema/SemaOverload.cpp (+2) - (modified) clang/lib/Serialization/ASTReaderStmt.cpp (+5) - (modified) clang/lib/Serialization/ASTWriterStmt.cpp (+2) ``````````diff diff --git a/clang/include/clang/AST/Expr.h b/clang/include/clang/AST/Expr.h index e9c3c16c87598..2a1b5a838d794 100644 --- a/clang/include/clang/AST/Expr.h +++ b/clang/include/clang/AST/Expr.h @@ -1344,7 +1344,13 @@ class DeclRefExpr final SourceLocation getLocation() const { return DeclRefExprBits.Loc; } void setLocation(SourceLocation L) { DeclRefExprBits.Loc = L; } - SourceLocation getBeginLoc() const LLVM_READONLY; + + SourceLocation getBeginLoc() const { + if (hasQualifier()) + return getQualifierLoc().getBeginLoc(); + return DeclRefExprBits.Loc; + } + SourceLocation getEndLoc() const LLVM_READONLY; /// Determine whether this declaration reference was preceded by a @@ -2901,34 +2907,44 @@ class CallExpr : public Expr { // // * An optional of type FPOptionsOverride. // - // Note that we store the offset in bytes from the this pointer to the start - // of the trailing objects. It would be perfectly possible to compute it - // based on the dynamic kind of the CallExpr. However 1.) we have plenty of - // space in the bit-fields of Stmt. 2.) It was benchmarked to be faster to - // compute this once and then load the offset from the bit-fields of Stmt, - // instead of re-computing the offset each time the trailing objects are - // accessed. + // CallExpr subclasses are asssumed to be 32 bytes or less, and CallExpr + // itself is 24 bytes. To avoid having to recompute or store the offset of the + // trailing objects, we put it at 32 bytes (such that it is suitable for all + // subclasses) We use the 8 bytes gap left for instances of CallExpr to store + // the begin and end source locations. Caching the begin source location in + // particular as a significant impact on perf as getBeginLoc is assumed to be + // cheap. + // The layourt is as follow: + // CallExpr | Begin | End | Trailing Objects + // CXXMemberCallExpr | Trailing Objects + // A bit in CallExprBitfields indicates if source locations are presents. +protected: + static constexpr unsigned offsetToTrailingObjects = 32; + template <typename T> + static constexpr unsigned + sizeToAllocateForCallExprSubclass(unsigned SizeOfTrailingObjects) { + static_assert(sizeof(T) <= CallExpr::offsetToTrailingObjects); + return SizeOfTrailingObjects + CallExpr::offsetToTrailingObjects; + } + +private: /// Return a pointer to the start of the trailing array of "Stmt *". Stmt **getTrailingStmts() { return reinterpret_cast<Stmt **>(reinterpret_cast<char *>(this) + - CallExprBits.OffsetToTrailingObjects); + offsetToTrailingObjects); } Stmt *const *getTrailingStmts() const { return const_cast<CallExpr *>(this)->getTrailingStmts(); } - /// Map a statement class to the appropriate offset in bytes from the - /// this pointer to the trailing objects. - static unsigned offsetToTrailingObjects(StmtClass SC); - unsigned getSizeOfTrailingStmts() const { return (1 + getNumPreArgs() + getNumArgs()) * sizeof(Stmt *); } size_t getOffsetOfTrailingFPFeatures() const { assert(hasStoredFPFeatures()); - return CallExprBits.OffsetToTrailingObjects + getSizeOfTrailingStmts(); + return offsetToTrailingObjects + getSizeOfTrailingStmts(); } public: @@ -2975,14 +2991,14 @@ class CallExpr : public Expr { FPOptionsOverride *getTrailingFPFeatures() { assert(hasStoredFPFeatures()); return reinterpret_cast<FPOptionsOverride *>( - reinterpret_cast<char *>(this) + CallExprBits.OffsetToTrailingObjects + + reinterpret_cast<char *>(this) + offsetToTrailingObjects + getSizeOfTrailingStmts()); } const FPOptionsOverride *getTrailingFPFeatures() const { assert(hasStoredFPFeatures()); return reinterpret_cast<const FPOptionsOverride *>( - reinterpret_cast<const char *>(this) + - CallExprBits.OffsetToTrailingObjects + getSizeOfTrailingStmts()); + reinterpret_cast<const char *>(this) + offsetToTrailingObjects + + getSizeOfTrailingStmts()); } public: @@ -3028,6 +3044,19 @@ class CallExpr : public Expr { bool hasStoredFPFeatures() const { return CallExprBits.HasFPFeatures; } + bool usesMemberSyntax() const { + return CallExprBits.ExplicitObjectMemFunUsingMemberSyntax; + } + void setUsesMemberSyntax(bool V = true) { + CallExprBits.ExplicitObjectMemFunUsingMemberSyntax = V; + // Because the source location may be different for explicit + // member, we reset the cached values. + if (CallExprBits.HasTrailingSourceLocs) { + CallExprBits.HasTrailingSourceLocs = false; + updateTrailingSourceLocs(); + } + } + bool isCoroElideSafe() const { return CallExprBits.IsCoroElideSafe; } void setCoroElideSafe(bool V = true) { CallExprBits.IsCoroElideSafe = V; } @@ -3187,9 +3216,59 @@ class CallExpr : public Expr { SourceLocation getRParenLoc() const { return RParenLoc; } void setRParenLoc(SourceLocation L) { RParenLoc = L; } - SourceLocation getBeginLoc() const LLVM_READONLY; - SourceLocation getEndLoc() const LLVM_READONLY; + template <unsigned N> SourceLocation getTrailingSourceLoc() const { + static_assert(N <= 1); + assert(CallExprBits.HasTrailingSourceLocs && "No trailing source loc"); + static_assert(sizeof(CallExpr) <= + offsetToTrailingObjects + 2 * sizeof(SourceLocation)); + return *reinterpret_cast<const SourceLocation *>( + reinterpret_cast<const char *>(this) + sizeof(CallExpr) + + sizeof(SourceLocation) * N); + } + + SourceLocation getBeginLoc() const { + if (CallExprBits.HasTrailingSourceLocs) + return getTrailingSourceLoc<0>(); + + if (usesMemberSyntax()) { + if (auto FirstArgLoc = getArg(0)->getBeginLoc(); FirstArgLoc.isValid()) { + return FirstArgLoc; + } + } + return getCallee()->getBeginLoc(); + } + + SourceLocation getEndLoc() const { + if (CallExprBits.HasTrailingSourceLocs) + return getTrailingSourceLoc<0>(); + + SourceLocation end = getRParenLoc(); + if (end.isInvalid() && getNumArgs() > 0 && getArg(getNumArgs() - 1)) + end = getArg(getNumArgs() - 1)->getEndLoc(); + return end; + } + +private: + bool hasTrailingSourceLoc() const { + return CallExprBits.HasTrailingSourceLocs; + } + void updateTrailingSourceLocs() { + assert(!CallExprBits.HasTrailingSourceLocs && + "Trailing source loc already set?"); + assert(getStmtClass() == CallExprClass && + "Calling setTrailingSourceLocs on a subclass of CallExpr"); + static_assert(sizeof(CallExpr) <= + offsetToTrailingObjects + 2 * sizeof(SourceLocation)); + + SourceLocation *Locs = reinterpret_cast<SourceLocation *>( + reinterpret_cast<char *>(this) + sizeof(CallExpr)); + new (Locs) SourceLocation(getBeginLoc()); + new (Locs + 1) SourceLocation(getEndLoc()); + CallExprBits.HasTrailingSourceLocs = true; + } + +public: /// Return true if this is a call to __assume() or __builtin_assume() with /// a non-value-dependent constant parameter evaluating as false. bool isBuiltinAssumeFalse(const ASTContext &Ctx) const; diff --git a/clang/include/clang/AST/NestedNameSpecifier.h b/clang/include/clang/AST/NestedNameSpecifier.h index d7da3272d0943..952c79753d10a 100644 --- a/clang/include/clang/AST/NestedNameSpecifier.h +++ b/clang/include/clang/AST/NestedNameSpecifier.h @@ -283,7 +283,9 @@ class NestedNameSpecifierLoc { /// For example, if this instance refers to a nested-name-specifier /// \c \::std::vector<int>::, the returned source range would cover /// from the initial '::' to the last '::'. - SourceRange getSourceRange() const LLVM_READONLY; + SourceRange getSourceRange() const LLVM_READONLY { + return SourceRange(getBeginLoc(), getEndLoc()); + } /// Retrieve the source range covering just the last part of /// this nested-name-specifier, not including the prefix. @@ -296,14 +298,18 @@ class NestedNameSpecifierLoc { /// Retrieve the location of the beginning of this /// nested-name-specifier. SourceLocation getBeginLoc() const { - return getSourceRange().getBegin(); + if (!Qualifier) + return SourceLocation(); + + NestedNameSpecifierLoc First = *this; + while (NestedNameSpecifierLoc Prefix = First.getPrefix()) + First = Prefix; + return First.getLocalSourceRange().getBegin(); } /// Retrieve the location of the end of this /// nested-name-specifier. - SourceLocation getEndLoc() const { - return getSourceRange().getEnd(); - } + SourceLocation getEndLoc() const { return getLocalSourceRange().getEnd(); } /// Retrieve the location of the beginning of this /// component of the nested-name-specifier. diff --git a/clang/include/clang/AST/Stmt.h b/clang/include/clang/AST/Stmt.h index 336eb6d3df7e1..e233f04b2405d 100644 --- a/clang/include/clang/AST/Stmt.h +++ b/clang/include/clang/AST/Stmt.h @@ -563,17 +563,21 @@ class alignas(void *) Stmt { unsigned HasFPFeatures : 1; /// True if the call expression is a must-elide call to a coroutine. + LLVM_PREFERRED_TYPE(bool) unsigned IsCoroElideSafe : 1; - /// Padding used to align OffsetToTrailingObjects to a byte multiple. - unsigned : 24 - 4 - NumExprBits; + /// Tracks When CallExpr is used to represent an explicit object + /// member function, in order to adjust the begin location. + LLVM_PREFERRED_TYPE(bool) + unsigned ExplicitObjectMemFunUsingMemberSyntax : 1; - /// The offset in bytes from the this pointer to the start of the - /// trailing objects belonging to CallExpr. Intentionally byte sized - /// for faster access. - unsigned OffsetToTrailingObjects : 8; + /// Indicates that SourceLocations are cached as + /// Trailing objects. See the definition of CallExpr. + LLVM_PREFERRED_TYPE(bool) + unsigned HasTrailingSourceLocs : 1; }; - enum { NumCallExprBits = 32 }; + + enum { NumCallExprBits = 25 }; class MemberExprBitfields { friend class ASTStmtReader; diff --git a/clang/lib/AST/Expr.cpp b/clang/lib/AST/Expr.cpp index fe874ccd7b60f..7f768665b3157 100644 --- a/clang/lib/AST/Expr.cpp +++ b/clang/lib/AST/Expr.cpp @@ -547,11 +547,6 @@ void DeclRefExpr::setDecl(ValueDecl *NewD) { setDependence(computeDependence(this, NewD->getASTContext())); } -SourceLocation DeclRefExpr::getBeginLoc() const { - if (hasQualifier()) - return getQualifierLoc().getBeginLoc(); - return getNameInfo().getBeginLoc(); -} SourceLocation DeclRefExpr::getEndLoc() const { if (hasExplicitTemplateArgs()) return getRAngleLoc(); @@ -1455,6 +1450,23 @@ OverloadedOperatorKind UnaryOperator::getOverloadedOperator(Opcode Opc) { // Postfix Operators. //===----------------------------------------------------------------------===// +static unsigned SizeOfCallExprInstance(Expr::StmtClass SC) { + switch (SC) { + case Expr::CallExprClass: + return sizeof(CallExpr); + case Expr::CXXOperatorCallExprClass: + return sizeof(CXXOperatorCallExpr); + case Expr::CXXMemberCallExprClass: + return sizeof(CXXMemberCallExpr); + case Expr::UserDefinedLiteralClass: + return sizeof(UserDefinedLiteral); + case Expr::CUDAKernelCallExprClass: + return sizeof(CUDAKernelCallExpr); + default: + llvm_unreachable("unexpected class deriving from CallExpr!"); + } +} + CallExpr::CallExpr(StmtClass SC, Expr *Fn, ArrayRef<Expr *> PreArgs, ArrayRef<Expr *> Args, QualType Ty, ExprValueKind VK, SourceLocation RParenLoc, FPOptionsOverride FPFeatures, @@ -1464,11 +1476,8 @@ CallExpr::CallExpr(StmtClass SC, Expr *Fn, ArrayRef<Expr *> PreArgs, unsigned NumPreArgs = PreArgs.size(); CallExprBits.NumPreArgs = NumPreArgs; assert((NumPreArgs == getNumPreArgs()) && "NumPreArgs overflow!"); - - unsigned OffsetToTrailingObjects = offsetToTrailingObjects(SC); - CallExprBits.OffsetToTrailingObjects = OffsetToTrailingObjects; - assert((CallExprBits.OffsetToTrailingObjects == OffsetToTrailingObjects) && - "OffsetToTrailingObjects overflow!"); + assert(SizeOfCallExprInstance(SC) <= offsetToTrailingObjects && + "This CallExpr subclass is too big or unsupported"); CallExprBits.UsesADL = static_cast<bool>(UsesADL); @@ -1484,6 +1493,9 @@ CallExpr::CallExpr(StmtClass SC, Expr *Fn, ArrayRef<Expr *> PreArgs, CallExprBits.HasFPFeatures = FPFeatures.requiresTrailingStorage(); CallExprBits.IsCoroElideSafe = false; + CallExprBits.ExplicitObjectMemFunUsingMemberSyntax = false; + CallExprBits.HasTrailingSourceLocs = false; + if (hasStoredFPFeatures()) setStoredFPFeatures(FPFeatures); } @@ -1493,13 +1505,10 @@ CallExpr::CallExpr(StmtClass SC, unsigned NumPreArgs, unsigned NumArgs, : Expr(SC, Empty), NumArgs(NumArgs) { CallExprBits.NumPreArgs = NumPreArgs; assert((NumPreArgs == getNumPreArgs()) && "NumPreArgs overflow!"); - - unsigned OffsetToTrailingObjects = offsetToTrailingObjects(SC); - CallExprBits.OffsetToTrailingObjects = OffsetToTrailingObjects; - assert((CallExprBits.OffsetToTrailingObjects == OffsetToTrailingObjects) && - "OffsetToTrailingObjects overflow!"); CallExprBits.HasFPFeatures = HasFPFeatures; CallExprBits.IsCoroElideSafe = false; + CallExprBits.ExplicitObjectMemFunUsingMemberSyntax = false; + CallExprBits.HasTrailingSourceLocs = false; } CallExpr *CallExpr::Create(const ASTContext &Ctx, Expr *Fn, @@ -1510,41 +1519,41 @@ CallExpr *CallExpr::Create(const ASTContext &Ctx, Expr *Fn, unsigned NumArgs = std::max<unsigned>(Args.size(), MinNumArgs); unsigned SizeOfTrailingObjects = CallExpr::sizeOfTrailingObjects( /*NumPreArgs=*/0, NumArgs, FPFeatures.requiresTrailingStorage()); - void *Mem = - Ctx.Allocate(sizeof(CallExpr) + SizeOfTrailingObjects, alignof(CallExpr)); - return new (Mem) CallExpr(CallExprClass, Fn, /*PreArgs=*/{}, Args, Ty, VK, - RParenLoc, FPFeatures, MinNumArgs, UsesADL); + void *Mem = Ctx.Allocate( + sizeToAllocateForCallExprSubclass<CallExpr>(SizeOfTrailingObjects), + alignof(CallExpr)); + CallExpr *E = + new (Mem) CallExpr(CallExprClass, Fn, /*PreArgs=*/{}, Args, Ty, VK, + RParenLoc, FPFeatures, MinNumArgs, UsesADL); + E->updateTrailingSourceLocs(); + return E; } CallExpr *CallExpr::CreateEmpty(const ASTContext &Ctx, unsigned NumArgs, bool HasFPFeatures, EmptyShell Empty) { unsigned SizeOfTrailingObjects = CallExpr::sizeOfTrailingObjects(/*NumPreArgs=*/0, NumArgs, HasFPFeatures); - void *Mem = - Ctx.Allocate(sizeof(CallExpr) + SizeOfTrailingObjects, alignof(CallExpr)); + void *Mem = Ctx.Allocate( + sizeToAllocateForCallExprSubclass<CallExpr>(SizeOfTrailingObjects), + alignof(CallExpr)); return new (Mem) CallExpr(CallExprClass, /*NumPreArgs=*/0, NumArgs, HasFPFeatures, Empty); } -unsigned CallExpr::offsetToTrailingObjects(StmtClass SC) { - switch (SC) { - case CallExprClass: - return sizeof(CallExpr); - case CXXOperatorCallExprClass: - return sizeof(CXXOperatorCallExpr); - case CXXMemberCallExprClass: - return sizeof(CXXMemberCallExpr); - case UserDefinedLiteralClass: - return sizeof(UserDefinedLiteral); - case CUDAKernelCallExprClass: - return sizeof(CUDAKernelCallExpr); - default: - llvm_unreachable("unexpected class deriving from CallExpr!"); - } -} - Decl *Expr::getReferencedDeclOfCallee() { - Expr *CEE = IgnoreParenImpCasts(); + + // Optimize for the common case first + // (simple function or member function call) + // then try more exotic possibilities + Expr *CEE = IgnoreImpCasts(); + + if (auto *DRE = dyn_cast<DeclRefExpr>(CEE)) + return DRE->getDecl(); + + if (auto *ME = dyn_cast<MemberExpr>(CEE)) + return ME->getMemberDecl(); + + CEE = CEE->IgnoreParens(); while (auto *NTTP = dyn_cast<SubstNonTypeTemplateParmExpr>(CEE)) CEE = NTTP->getReplacement()->IgnoreParenImpCasts(); @@ -1638,43 +1647,6 @@ CallExpr::getUnusedResultAttr(const ASTContext &Ctx) const { return {nullptr, nullptr}; } -SourceLocation CallExpr::getBeginLoc() const { - if (const auto *OCE = dyn_cast<CXXOperatorCallExpr>(this)) - return OCE->getBeginLoc(); - - // A non-dependent call to a member function with an explicit object parameter - // is modelled with the object expression being the first argument, e.g. in - // `o.f(x)`, the callee will be just `f`, and `o` will be the first argument. - // Since the first argument is written before the callee, the expression's - // begin location should come from the first argument. - // This does not apply to dependent calls, which are modelled with `o.f` - // being the callee. - if (!isTypeDependent()) { - if (const auto *Method = - dyn_cast_if_present<const CXXMethodDecl>(getCalleeDecl()); - Method && Method->isExplicitObjectMemberFunction()) { - if (auto FirstArgLoc = getArg(0)->getBeginLoc(); FirstArgLoc.isValid()) { - return FirstArgLoc; - } - } - } - - SourceLocation begin = getCallee()->getBeginLoc(); - if (begin.isInvalid() && getNumArgs() > 0 && getArg(0)) - begin = getArg(0)->getBeginLoc(); - return begin; -} - -SourceLocation CallExpr::getEndLoc() const { - if (const auto *OCE = dyn_cast<CXXOperatorCallExpr>(this)) - return OCE->getEndLoc(); - - SourceLocation end = getRParenLoc(); - if (end.isInvalid() && getNumArgs() > 0 && getArg(getNumArgs() - 1)) - end = getArg(getNumArgs() - 1)->getEndLoc(); - return end; -} - OffsetOfExpr *OffsetOfExpr::Create(const ASTContext &C, QualType type, SourceLocation OperatorLoc, TypeSourceInfo *tsi, diff --git a/clang/lib/AST/ExprCXX.cpp b/clang/lib/AST/ExprCXX.cpp index 00bddce3a1ee2..5c712e146e5a8 100644 --- a/clang/lib/AST/ExprCXX.cpp +++ b/clang/lib/AST/ExprCXX.cpp @@ -619,8 +619,10 @@ CXXOperatorCallExpr::Create(const ASTContext &Ctx, unsigned NumArgs = Args.size(); unsigned SizeOfTrailingObjects = CallExpr::sizeOfTrailingObjects( /*NumPreArgs=*/0, NumArgs, FPFeatures.requiresTrailingStorage()); - void *Mem = Ctx.Allocate(sizeof(CXXOperatorCallExpr) + SizeOfTrailingObjects, - alignof(CXXOperatorCallExpr)); + void *Mem = + Ctx.Allocate(sizeToAllocateForCallExprSubclass<CXXOperatorCallExpr>( + SizeOfTrailingObjects), + alignof(CXXOperatorCallExpr)); return new (Mem) CXXOperatorCallExpr(OpKind, Fn, Args, Ty, VK, OperatorLoc, FPFeatures, UsesADL); } @@ -632,8 +634,10 @@ CXXOperatorCallExpr *CXXOperatorCallExpr::CreateEmpty(const ASTContext &Ctx, // Allocate storage for the trailing objects of CallExpr. unsigned SizeOfTrailingObjects = CallExpr::sizeOfTrailingObjects(/*NumPreArgs=*/0, NumArgs, HasFPFeatures); - void *Mem = Ctx.Allocate(sizeof(CXXOperatorCallExpr) + SizeOfTrailingObjects, - alignof(CXXOperatorCallExpr)); + void *Mem = + Ctx.Allocate(sizeToAllocateForCallExprSubclass<CXXOperatorCallExpr>( + SizeOfTrailingObjects), + alignof(CXXOperatorCallExpr)); return new (Mem) CXXOperatorCallExpr(NumArgs, HasFPFeatures, Empty); } @@ -684,7 +688,8 @@ CXXMemberCallExpr *CXXMemberCallExpr::Create(const ASTContext &Ctx, Expr *Fn, unsigned NumArgs = std::max<unsigned>(Args.size(), MinNumArgs); unsigned SizeOfTrailingObjects = CallExpr::sizeOfTrailingObjects( /*NumPreArgs=*/0, NumArgs, FPFeatures.requiresTrailingStorage()); - void *Mem = Ctx.Allocate(sizeof(CXXMemberCallExpr) + SizeOfTrailingObjects, + void *Mem = Ctx.Allocate(sizeToAllocateForCallExprSubclass<CXXMemberCallExpr>( + SizeOfTrailingObjects), alignof(CXXMemberCallExpr)); return new (Mem) CXXMemberCallExpr(Fn, Args, Ty, VK, RP, FPFeatures, MinNumArgs); @@ -697,7 +702,8 @@ CXXMemberCallExpr *CXXMemberCallExpr::CreateEmpty(const ASTContext &Ctx, // Allocate storage for the trailing objects of CallExpr. unsigned SizeOfTrailingObjects = CallExpr::sizeOfTrailingObjects(/*NumPreArgs=*/0, NumArgs, HasFPFeatures); - void *Mem = Ctx.Allocate(sizeof(CXXMemberCallExpr) + SizeOfTrailingObjects, + void *Mem = Ctx.Allocate(sizeToAllocateForCallExprSubclass<... [truncated] `````````` </details> https://github.com/llvm/llvm-project/pull/141058 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
