llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-clang Author: Vitaly Buka (vitalybuka) <details> <summary>Changes</summary> Reverts llvm/llvm-project#<!-- -->88546 Leak and performance regression. Details in #<!-- -->88546 --- Full diff: https://github.com/llvm/llvm-project/pull/89006.diff 5 Files Affected: - (modified) clang/docs/ReleaseNotes.rst (-6) - (modified) clang/include/clang/Sema/Initialization.h (+2-4) - (modified) clang/include/clang/Sema/Overload.h (+51-19) - (modified) clang/lib/Sema/SemaInit.cpp (+12-14) - (modified) clang/lib/Sema/SemaOverload.cpp (+10-11) ``````````diff diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst index 4aedfafcb26aea..3752b6ce157600 100644 --- a/clang/docs/ReleaseNotes.rst +++ b/clang/docs/ReleaseNotes.rst @@ -203,12 +203,6 @@ Non-comprehensive list of changes in this release - ``__typeof_unqual__`` is available in all C modes as an extension, which behaves like ``typeof_unqual`` from C23, similar to ``__typeof__`` and ``typeof``. -- Improved stack usage with C++ initialization code. This allows significantly - more levels of recursive initialization before reaching stack exhaustion - limits. This will positively impact recursive template instantiation code, - but should also reduce memory overhead for initializations in general. - Fixes #GH88330 - New Compiler Flags ------------------ - ``-fsanitize=implicit-bitfield-conversion`` checks implicit truncation and diff --git a/clang/include/clang/Sema/Initialization.h b/clang/include/clang/Sema/Initialization.h index 1ceacf0f49f568..2072cd8d1c3ef8 100644 --- a/clang/include/clang/Sema/Initialization.h +++ b/clang/include/clang/Sema/Initialization.h @@ -1134,7 +1134,7 @@ class InitializationSequence { OverloadingResult FailedOverloadResult; /// The candidate set created when initialization failed. - std::unique_ptr<OverloadCandidateSet> FailedCandidateSet; + OverloadCandidateSet FailedCandidateSet; /// The incomplete type that caused a failure. QualType FailedIncompleteType; @@ -1403,9 +1403,7 @@ class InitializationSequence { /// Retrieve a reference to the candidate set when overload /// resolution fails. OverloadCandidateSet &getFailedCandidateSet() { - assert(FailedCandidateSet && - "this should have been allocated in the constructor!"); - return *FailedCandidateSet; + return FailedCandidateSet; } /// Get the overloading result, for when the initialization diff --git a/clang/include/clang/Sema/Overload.h b/clang/include/clang/Sema/Overload.h index e6f88bbf7c4f47..76311b00d2fc58 100644 --- a/clang/include/clang/Sema/Overload.h +++ b/clang/include/clang/Sema/Overload.h @@ -37,7 +37,6 @@ #include <cassert> #include <cstddef> #include <cstdint> -#include <memory> #include <utility> namespace clang { @@ -875,8 +874,7 @@ class Sema; ConversionFixItGenerator Fix; /// Viable - True to indicate that this overload candidate is viable. - LLVM_PREFERRED_TYPE(bool) - unsigned Viable : 1; + bool Viable : 1; /// Whether this candidate is the best viable function, or tied for being /// the best viable function. @@ -885,14 +883,12 @@ class Sema; /// was part of the ambiguity kernel: the minimal non-empty set of viable /// candidates such that all elements of the ambiguity kernel are better /// than all viable candidates not in the ambiguity kernel. - LLVM_PREFERRED_TYPE(bool) - unsigned Best : 1; + bool Best : 1; /// IsSurrogate - True to indicate that this candidate is a /// surrogate for a conversion to a function pointer or reference /// (C++ [over.call.object]). - LLVM_PREFERRED_TYPE(bool) - unsigned IsSurrogate : 1; + bool IsSurrogate : 1; /// IgnoreObjectArgument - True to indicate that the first /// argument's conversion, which for this function represents the @@ -901,20 +897,18 @@ class Sema; /// implicit object argument is just a placeholder) or a /// non-static member function when the call doesn't have an /// object argument. - LLVM_PREFERRED_TYPE(bool) - unsigned IgnoreObjectArgument : 1; + bool IgnoreObjectArgument : 1; /// True if the candidate was found using ADL. - LLVM_PREFERRED_TYPE(CallExpr::ADLCallKind) - unsigned IsADLCandidate : 1; + CallExpr::ADLCallKind IsADLCandidate : 1; /// Whether this is a rewritten candidate, and if so, of what kind? LLVM_PREFERRED_TYPE(OverloadCandidateRewriteKind) unsigned RewriteKind : 2; /// FailureKind - The reason why this candidate is not viable. - LLVM_PREFERRED_TYPE(OverloadFailureKind) - unsigned FailureKind : 5; + /// Actually an OverloadFailureKind. + unsigned char FailureKind; /// The number of call arguments that were explicitly provided, /// to be used while performing partial ordering of function templates. @@ -978,9 +972,7 @@ class Sema; private: friend class OverloadCandidateSet; OverloadCandidate() - : IsSurrogate(false), - IsADLCandidate(static_cast<unsigned>(CallExpr::NotADL)), - RewriteKind(CRK_None) {} + : IsSurrogate(false), IsADLCandidate(CallExpr::NotADL), RewriteKind(CRK_None) {} }; /// OverloadCandidateSet - A set of overload candidates, used in C++ @@ -1078,16 +1070,51 @@ class Sema; }; private: - SmallVector<OverloadCandidate, 4> Candidates; - llvm::SmallPtrSet<uintptr_t, 4> Functions; + SmallVector<OverloadCandidate, 16> Candidates; + llvm::SmallPtrSet<uintptr_t, 16> Functions; + + // Allocator for ConversionSequenceLists. We store the first few of these + // inline to avoid allocation for small sets. + llvm::BumpPtrAllocator SlabAllocator; SourceLocation Loc; CandidateSetKind Kind; OperatorRewriteInfo RewriteInfo; + constexpr static unsigned NumInlineBytes = + 24 * sizeof(ImplicitConversionSequence); + unsigned NumInlineBytesUsed = 0; + alignas(void *) char InlineSpace[NumInlineBytes]; + // Address space of the object being constructed. LangAS DestAS = LangAS::Default; + /// If we have space, allocates from inline storage. Otherwise, allocates + /// from the slab allocator. + /// FIXME: It would probably be nice to have a SmallBumpPtrAllocator + /// instead. + /// FIXME: Now that this only allocates ImplicitConversionSequences, do we + /// want to un-generalize this? + template <typename T> + T *slabAllocate(unsigned N) { + // It's simpler if this doesn't need to consider alignment. + static_assert(alignof(T) == alignof(void *), + "Only works for pointer-aligned types."); + static_assert(std::is_trivial<T>::value || + std::is_same<ImplicitConversionSequence, T>::value, + "Add destruction logic to OverloadCandidateSet::clear()."); + + unsigned NBytes = sizeof(T) * N; + if (NBytes > NumInlineBytes - NumInlineBytesUsed) + return SlabAllocator.Allocate<T>(N); + char *FreeSpaceStart = InlineSpace + NumInlineBytesUsed; + assert(uintptr_t(FreeSpaceStart) % alignof(void *) == 0 && + "Misaligned storage!"); + + NumInlineBytesUsed += NBytes; + return reinterpret_cast<T *>(FreeSpaceStart); + } + void destroyCandidates(); public: @@ -1136,7 +1163,12 @@ class Sema; ConversionSequenceList allocateConversionSequences(unsigned NumConversions) { ImplicitConversionSequence *Conversions = - new ImplicitConversionSequence[NumConversions]; + slabAllocate<ImplicitConversionSequence>(NumConversions); + + // Construct the new objects. + for (unsigned I = 0; I != NumConversions; ++I) + new (&Conversions[I]) ImplicitConversionSequence(); + return ConversionSequenceList(Conversions, NumConversions); } diff --git a/clang/lib/Sema/SemaInit.cpp b/clang/lib/Sema/SemaInit.cpp index 791c0b6e6df23e..fb7a80ab02846c 100644 --- a/clang/lib/Sema/SemaInit.cpp +++ b/clang/lib/Sema/SemaInit.cpp @@ -6114,8 +6114,7 @@ InitializationSequence::InitializationSequence( Sema &S, const InitializedEntity &Entity, const InitializationKind &Kind, MultiExprArg Args, bool TopLevelOfInitList, bool TreatUnavailableAsInvalid) : FailedOverloadResult(OR_Success), - FailedCandidateSet(new OverloadCandidateSet( - Kind.getLocation(), OverloadCandidateSet::CSK_Normal)) { + FailedCandidateSet(Kind.getLocation(), OverloadCandidateSet::CSK_Normal) { InitializeFrom(S, Entity, Kind, Args, TopLevelOfInitList, TreatUnavailableAsInvalid); } @@ -9736,7 +9735,7 @@ bool InitializationSequence::Diagnose(Sema &S, switch (FailedOverloadResult) { case OR_Ambiguous: - FailedCandidateSet->NoteCandidates( + FailedCandidateSet.NoteCandidates( PartialDiagnosticAt( Kind.getLocation(), Failure == FK_UserConversionOverloadFailed @@ -9750,8 +9749,7 @@ bool InitializationSequence::Diagnose(Sema &S, break; case OR_No_Viable_Function: { - auto Cands = - FailedCandidateSet->CompleteCandidates(S, OCD_AllCandidates, Args); + auto Cands = FailedCandidateSet.CompleteCandidates(S, OCD_AllCandidates, Args); if (!S.RequireCompleteType(Kind.getLocation(), DestType.getNonReferenceType(), diag::err_typecheck_nonviable_condition_incomplete, @@ -9761,13 +9759,13 @@ bool InitializationSequence::Diagnose(Sema &S, << OnlyArg->getType() << Args[0]->getSourceRange() << DestType.getNonReferenceType(); - FailedCandidateSet->NoteCandidates(S, Args, Cands); + FailedCandidateSet.NoteCandidates(S, Args, Cands); break; } case OR_Deleted: { OverloadCandidateSet::iterator Best; - OverloadingResult Ovl = - FailedCandidateSet->BestViableFunction(S, Kind.getLocation(), Best); + OverloadingResult Ovl + = FailedCandidateSet.BestViableFunction(S, Kind.getLocation(), Best); StringLiteral *Msg = Best->Function->getDeletedMessage(); S.Diag(Kind.getLocation(), diag::err_typecheck_deleted_function) @@ -9951,7 +9949,7 @@ bool InitializationSequence::Diagnose(Sema &S, // bad. switch (FailedOverloadResult) { case OR_Ambiguous: - FailedCandidateSet->NoteCandidates( + FailedCandidateSet.NoteCandidates( PartialDiagnosticAt(Kind.getLocation(), S.PDiag(diag::err_ovl_ambiguous_init) << DestType << ArgsRange), @@ -10005,7 +10003,7 @@ bool InitializationSequence::Diagnose(Sema &S, break; } - FailedCandidateSet->NoteCandidates( + FailedCandidateSet.NoteCandidates( PartialDiagnosticAt( Kind.getLocation(), S.PDiag(diag::err_ovl_no_viable_function_in_init) @@ -10015,8 +10013,8 @@ bool InitializationSequence::Diagnose(Sema &S, case OR_Deleted: { OverloadCandidateSet::iterator Best; - OverloadingResult Ovl = - FailedCandidateSet->BestViableFunction(S, Kind.getLocation(), Best); + OverloadingResult Ovl + = FailedCandidateSet.BestViableFunction(S, Kind.getLocation(), Best); if (Ovl != OR_Deleted) { S.Diag(Kind.getLocation(), diag::err_ovl_deleted_init) << DestType << ArgsRange; @@ -10095,8 +10093,8 @@ bool InitializationSequence::Diagnose(Sema &S, S.Diag(Kind.getLocation(), diag::err_selected_explicit_constructor) << Args[0]->getSourceRange(); OverloadCandidateSet::iterator Best; - OverloadingResult Ovl = - FailedCandidateSet->BestViableFunction(S, Kind.getLocation(), Best); + OverloadingResult Ovl + = FailedCandidateSet.BestViableFunction(S, Kind.getLocation(), Best); (void)Ovl; assert(Ovl == OR_Success && "Inconsistent overload resolution"); CXXConstructorDecl *CtorDecl = cast<CXXConstructorDecl>(Best->Function); diff --git a/clang/lib/Sema/SemaOverload.cpp b/clang/lib/Sema/SemaOverload.cpp index bcde0d86cf10fd..227ef564ba3e08 100644 --- a/clang/lib/Sema/SemaOverload.cpp +++ b/clang/lib/Sema/SemaOverload.cpp @@ -1057,7 +1057,8 @@ bool OverloadCandidateSet::OperatorRewriteInfo::shouldAddReversed( void OverloadCandidateSet::destroyCandidates() { for (iterator i = begin(), e = end(); i != e; ++i) { - delete[] i->Conversions.data(); + for (auto &C : i->Conversions) + C.~ImplicitConversionSequence(); if (!i->Viable && i->FailureKind == ovl_fail_bad_deduction) i->DeductionFailure.Destroy(); } @@ -1065,6 +1066,8 @@ void OverloadCandidateSet::destroyCandidates() { void OverloadCandidateSet::clear(CandidateSetKind CSK) { destroyCandidates(); + SlabAllocator.Reset(); + NumInlineBytesUsed = 0; Candidates.clear(); Functions.clear(); Kind = CSK; @@ -6980,7 +6983,7 @@ void Sema::AddOverloadCandidate( Candidate.RewriteKind = CandidateSet.getRewriteInfo().getRewriteKind(Function, PO); Candidate.IsSurrogate = false; - Candidate.IsADLCandidate = static_cast<unsigned>(IsADLCandidate); + Candidate.IsADLCandidate = IsADLCandidate; Candidate.IgnoreObjectArgument = false; Candidate.ExplicitCallArguments = Args.size(); @@ -7812,7 +7815,7 @@ void Sema::AddTemplateOverloadCandidate( Candidate.RewriteKind = CandidateSet.getRewriteInfo().getRewriteKind(Candidate.Function, PO); Candidate.IsSurrogate = false; - Candidate.IsADLCandidate = static_cast<unsigned>(IsADLCandidate); + Candidate.IsADLCandidate = IsADLCandidate; // Ignore the object argument if there is one, since we don't have an object // type. Candidate.IgnoreObjectArgument = @@ -14122,8 +14125,7 @@ static ExprResult FinishOverloadedCallExpr(Sema &SemaRef, Scope *S, Expr *Fn, return ExprError(); return SemaRef.BuildResolvedCallExpr( Res.get(), FDecl, LParenLoc, Args, RParenLoc, ExecConfig, - /*IsExecConfig=*/false, - static_cast<CallExpr::ADLCallKind>((*Best)->IsADLCandidate)); + /*IsExecConfig=*/false, (*Best)->IsADLCandidate); } case OR_No_Viable_Function: { @@ -14182,8 +14184,7 @@ static ExprResult FinishOverloadedCallExpr(Sema &SemaRef, Scope *S, Expr *Fn, return ExprError(); return SemaRef.BuildResolvedCallExpr( Res.get(), FDecl, LParenLoc, Args, RParenLoc, ExecConfig, - /*IsExecConfig=*/false, - static_cast<CallExpr::ADLCallKind>((*Best)->IsADLCandidate)); + /*IsExecConfig=*/false, (*Best)->IsADLCandidate); } } @@ -14490,8 +14491,7 @@ Sema::CreateOverloadedUnaryOp(SourceLocation OpLoc, UnaryOperatorKind Opc, Args[0] = Input; CallExpr *TheCall = CXXOperatorCallExpr::Create( Context, Op, FnExpr.get(), ArgsArray, ResultTy, VK, OpLoc, - CurFPFeatureOverrides(), - static_cast<CallExpr::ADLCallKind>(Best->IsADLCandidate)); + CurFPFeatureOverrides(), Best->IsADLCandidate); if (CheckCallReturnType(FnDecl->getReturnType(), OpLoc, TheCall, FnDecl)) return ExprError(); @@ -14909,8 +14909,7 @@ ExprResult Sema::CreateOverloadedBinOp(SourceLocation OpLoc, // members; CodeGen should take care not to emit the this pointer. TheCall = CXXOperatorCallExpr::Create( Context, ChosenOp, FnExpr.get(), Args, ResultTy, VK, OpLoc, - CurFPFeatureOverrides(), - static_cast<CallExpr::ADLCallKind>(Best->IsADLCandidate)); + CurFPFeatureOverrides(), Best->IsADLCandidate); if (const auto *Method = dyn_cast<CXXMethodDecl>(FnDecl); Method && Method->isImplicitObjectMemberFunction()) { `````````` </details> https://github.com/llvm/llvm-project/pull/89006 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits