[clang] Revert "Improve stack usage to increase recursive initialization depth" (PR #89006)
Vitaly Buka via cfe-commits
cfe-commits at lists.llvm.org
Tue Apr 16 17:32:14 PDT 2024
https://github.com/vitalybuka updated https://github.com/llvm/llvm-project/pull/89006
>From 1d59298cd9ed21e1ac860d64f965514a577f45bb Mon Sep 17 00:00:00 2001
From: Vitaly Buka <vitalybuka at gmail.com>
Date: Tue, 16 Apr 2024 17:23:47 -0700
Subject: [PATCH 1/2] Revert "Improve stack usage to increase recursive
initialization depth (#88546)"
This reverts commit 4082a7554521572a65a5a0008c4661a534df659d.
---
clang/docs/ReleaseNotes.rst | 6 --
clang/include/clang/Sema/Initialization.h | 6 +-
clang/include/clang/Sema/Overload.h | 70 +++++++++++++++++------
clang/lib/Sema/SemaInit.cpp | 26 ++++-----
clang/lib/Sema/SemaOverload.cpp | 21 ++++---
5 files changed, 75 insertions(+), 54 deletions(-)
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()) {
>From b9f0b0e8b44018ef0d82252bc55830f165f0f5c4 Mon Sep 17 00:00:00 2001
From: Vitaly Buka <vitalybuka at google.com>
Date: Tue, 16 Apr 2024 17:31:38 -0700
Subject: [PATCH 2/2] clang-format
---
clang/include/clang/Sema/Initialization.h | 4 +---
clang/include/clang/Sema/Overload.h | 6 +++---
clang/lib/Sema/SemaInit.cpp | 15 ++++++++-------
3 files changed, 12 insertions(+), 13 deletions(-)
diff --git a/clang/include/clang/Sema/Initialization.h b/clang/include/clang/Sema/Initialization.h
index 2072cd8d1c3ef8..0cae6d353d22bc 100644
--- a/clang/include/clang/Sema/Initialization.h
+++ b/clang/include/clang/Sema/Initialization.h
@@ -1402,9 +1402,7 @@ class InitializationSequence {
/// Retrieve a reference to the candidate set when overload
/// resolution fails.
- OverloadCandidateSet &getFailedCandidateSet() {
- return FailedCandidateSet;
- }
+ OverloadCandidateSet &getFailedCandidateSet() { return FailedCandidateSet; }
/// Get the overloading result, for when the initialization
/// sequence failed due to a bad overload.
diff --git a/clang/include/clang/Sema/Overload.h b/clang/include/clang/Sema/Overload.h
index 76311b00d2fc58..a49cc6778d46b0 100644
--- a/clang/include/clang/Sema/Overload.h
+++ b/clang/include/clang/Sema/Overload.h
@@ -972,7 +972,8 @@ class Sema;
private:
friend class OverloadCandidateSet;
OverloadCandidate()
- : IsSurrogate(false), IsADLCandidate(CallExpr::NotADL), RewriteKind(CRK_None) {}
+ : IsSurrogate(false), IsADLCandidate(CallExpr::NotADL),
+ RewriteKind(CRK_None) {}
};
/// OverloadCandidateSet - A set of overload candidates, used in C++
@@ -1095,8 +1096,7 @@ class Sema;
/// instead.
/// FIXME: Now that this only allocates ImplicitConversionSequences, do we
/// want to un-generalize this?
- template <typename T>
- T *slabAllocate(unsigned N) {
+ 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.");
diff --git a/clang/lib/Sema/SemaInit.cpp b/clang/lib/Sema/SemaInit.cpp
index fb7a80ab02846c..6afcc56c73540b 100644
--- a/clang/lib/Sema/SemaInit.cpp
+++ b/clang/lib/Sema/SemaInit.cpp
@@ -9749,7 +9749,8 @@ 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,
@@ -9764,8 +9765,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);
StringLiteral *Msg = Best->Function->getDeletedMessage();
S.Diag(Kind.getLocation(), diag::err_typecheck_deleted_function)
@@ -10013,8 +10014,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;
@@ -10093,8 +10094,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);
More information about the cfe-commits
mailing list