[clang] Performance optimizations for function effects (nonblocking attribute etc.) (PR #96844)
Doug Wyatt via cfe-commits
cfe-commits at lists.llvm.org
Fri Jul 12 08:53:29 PDT 2024
https://github.com/dougsonos updated https://github.com/llvm/llvm-project/pull/96844
>From 038b39d3235c6c8151127c34d34f498dd298273c Mon Sep 17 00:00:00 2001
From: Doug Wyatt <dwyatt at apple.com>
Date: Wed, 26 Jun 2024 10:03:25 -0700
Subject: [PATCH 1/2] Performance optimizations for function effects: - Put new
FunctionProtoType trailing objects last. - Inline FunctionEffectsRef::get() -
Manually inline FunctionEffectsRef::Profile().
---
clang/include/clang/AST/Type.h | 30 +++++++++++++++++++--------
clang/lib/AST/ASTContext.cpp | 11 +++++-----
clang/lib/AST/Type.cpp | 37 +++++++++++-----------------------
clang/lib/Sema/SemaType.cpp | 1 +
4 files changed, 41 insertions(+), 38 deletions(-)
diff --git a/clang/include/clang/AST/Type.h b/clang/include/clang/AST/Type.h
index 62836ec5c6312..4545ad94d522a 100644
--- a/clang/include/clang/AST/Type.h
+++ b/clang/include/clang/AST/Type.h
@@ -132,7 +132,6 @@ class TemplateArgument;
class TemplateArgumentListInfo;
class TemplateArgumentLoc;
class TemplateTypeParmDecl;
-template <typename> class TreeTransform;
class TypedefNameDecl;
class UnresolvedUsingTypenameDecl;
class UsingShadowDecl;
@@ -4899,7 +4898,6 @@ class FunctionEffectsRef {
return !(LHS == RHS);
}
- void Profile(llvm::FoldingSetNodeID &ID) const;
void dump(llvm::raw_ostream &OS) const;
};
@@ -4970,7 +4968,7 @@ class FunctionProtoType final
FunctionType::FunctionTypeExtraBitfields,
FunctionType::FunctionTypeArmAttributes, FunctionType::ExceptionType,
Expr *, FunctionDecl *, FunctionType::ExtParameterInfo,
- FunctionEffect, EffectConditionExpr, Qualifiers> {
+ Qualifiers, FunctionEffect, EffectConditionExpr> {
friend class ASTContext; // ASTContext creates these.
friend TrailingObjects;
@@ -5001,21 +4999,21 @@ class FunctionProtoType final
// an ExtParameterInfo for each of the parameters. Present if and
// only if hasExtParameterInfos() is true.
//
+ // * Optionally a Qualifiers object to represent extra qualifiers that can't
+ // be represented by FunctionTypeBitfields.FastTypeQuals. Present if and
+ // only if hasExtQualifiers() is true.
+ //
// * Optionally, an array of getNumFunctionEffects() FunctionEffect.
// Present only when getNumFunctionEffects() > 0
//
// * Optionally, an array of getNumFunctionEffects() EffectConditionExpr.
// Present only when getNumFunctionEffectConditions() > 0.
//
- // * Optionally a Qualifiers object to represent extra qualifiers that can't
- // be represented by FunctionTypeBitfields.FastTypeQuals. Present if and
- // only if hasExtQualifiers() is true.
- //
// The optional FunctionTypeExtraBitfields has to be before the data
// related to the exception specification since it contains the number
// of exception types.
//
- // We put the ExtParameterInfos last. If all were equal, it would make
+ // We put the ExtParameterInfos later. If all were equal, it would make
// more sense to put these before the exception specification, because
// it's much easier to skip past them compared to the elaborate switch
// required to skip the exception specification. However, all is not
@@ -5132,6 +5130,10 @@ class FunctionProtoType final
return hasExtParameterInfos() ? getNumParams() : 0;
}
+ unsigned numTrailingObjects(OverloadToken<Qualifiers>) const {
+ return hasExtQualifiers() ? 1 : 0;
+ }
+
unsigned numTrailingObjects(OverloadToken<FunctionEffect>) const {
return getNumFunctionEffects();
}
@@ -8619,6 +8621,18 @@ QualType DecayedType::getPointeeType() const {
void FixedPointValueToString(SmallVectorImpl<char> &Str, llvm::APSInt Val,
unsigned Scale);
+inline FunctionEffectsRef FunctionEffectsRef::get(QualType QT) {
+ while (true) {
+ QualType Pointee = QT->getPointeeType();
+ if (Pointee.isNull())
+ break;
+ QT = Pointee;
+ }
+ if (const auto *FPT = QT->getAs<FunctionProtoType>())
+ return FPT->getFunctionEffects();
+ return {};
+}
+
} // namespace clang
#endif // LLVM_CLANG_AST_TYPE_H
diff --git a/clang/lib/AST/ASTContext.cpp b/clang/lib/AST/ASTContext.cpp
index 1b5d16bd176f3..b5519b3fc25a3 100644
--- a/clang/lib/AST/ASTContext.cpp
+++ b/clang/lib/AST/ASTContext.cpp
@@ -4601,14 +4601,15 @@ QualType ASTContext::getFunctionTypeInternal(
size_t Size = FunctionProtoType::totalSizeToAlloc<
QualType, SourceLocation, FunctionType::FunctionTypeExtraBitfields,
FunctionType::FunctionTypeArmAttributes, FunctionType::ExceptionType,
- Expr *, FunctionDecl *, FunctionProtoType::ExtParameterInfo,
- FunctionEffect, EffectConditionExpr, Qualifiers>(
+ Expr *, FunctionDecl *, FunctionProtoType::ExtParameterInfo, Qualifiers,
+ FunctionEffect, EffectConditionExpr>(
NumArgs, EPI.Variadic, EPI.requiresFunctionProtoTypeExtraBitfields(),
EPI.requiresFunctionProtoTypeArmAttributes(), ESH.NumExceptionType,
ESH.NumExprPtr, ESH.NumFunctionDeclPtr,
- EPI.ExtParameterInfos ? NumArgs : 0, EPI.FunctionEffects.size(),
- EPI.FunctionEffects.conditions().size(),
- EPI.TypeQuals.hasNonFastQualifiers() ? 1 : 0);
+ EPI.ExtParameterInfos ? NumArgs : 0,
+ EPI.TypeQuals.hasNonFastQualifiers() ? 1 : 0,
+ EPI.FunctionEffects.size(),
+ EPI.FunctionEffects.conditions().size());
auto *FTP = (FunctionProtoType *)Allocate(Size, alignof(FunctionProtoType));
FunctionProtoType::ExtProtoInfo newEPI = EPI;
diff --git a/clang/lib/AST/Type.cpp b/clang/lib/AST/Type.cpp
index d8b885870de3a..466928bcb9c0d 100644
--- a/clang/lib/AST/Type.cpp
+++ b/clang/lib/AST/Type.cpp
@@ -3857,9 +3857,19 @@ void FunctionProtoType::Profile(llvm::FoldingSetNodeID &ID, QualType Result,
}
epi.ExtInfo.Profile(ID);
- ID.AddInteger((epi.AArch64SMEAttributes << 1) | epi.HasTrailingReturn);
- epi.FunctionEffects.Profile(ID);
+ unsigned EffectCount = epi.FunctionEffects.size();
+ bool HasConds = !epi.FunctionEffects.Conditions.empty();
+
+ ID.AddInteger(
+ (EffectCount << 3) | (HasConds << 2) |
+ (epi.AArch64SMEAttributes << 1) | epi.HasTrailingReturn);
+
+ for (unsigned Idx = 0; Idx != EffectCount; ++Idx) {
+ ID.AddInteger(epi.FunctionEffects.Effects[Idx].toOpaqueInt32());
+ if (HasConds)
+ ID.AddPointer(epi.FunctionEffects.Conditions[Idx].getCondition());
+ }
}
void FunctionProtoType::Profile(llvm::FoldingSetNodeID &ID,
@@ -5219,17 +5229,6 @@ bool FunctionEffect::shouldDiagnoseFunctionCall(
// =====
-void FunctionEffectsRef::Profile(llvm::FoldingSetNodeID &ID) const {
- bool HasConds = !Conditions.empty();
-
- ID.AddInteger(size() | (HasConds << 31u));
- for (unsigned Idx = 0, Count = Effects.size(); Idx != Count; ++Idx) {
- ID.AddInteger(Effects[Idx].toOpaqueInt32());
- if (HasConds)
- ID.AddPointer(Conditions[Idx].getCondition());
- }
-}
-
bool FunctionEffectSet::insert(const FunctionEffectWithCondition &NewEC,
Conflicts &Errs) {
FunctionEffect::Kind NewOppositeKind = NewEC.Effect.oppositeKind();
@@ -5351,18 +5350,6 @@ LLVM_DUMP_METHOD void FunctionEffectSet::dump(llvm::raw_ostream &OS) const {
FunctionEffectsRef(*this).dump(OS);
}
-FunctionEffectsRef FunctionEffectsRef::get(QualType QT) {
- while (true) {
- QualType Pointee = QT->getPointeeType();
- if (Pointee.isNull())
- break;
- QT = Pointee;
- }
- if (const auto *FPT = QT->getAs<FunctionProtoType>())
- return FPT->getFunctionEffects();
- return {};
-}
-
FunctionEffectsRef
FunctionEffectsRef::create(ArrayRef<FunctionEffect> FX,
ArrayRef<EffectConditionExpr> Conds) {
diff --git a/clang/lib/Sema/SemaType.cpp b/clang/lib/Sema/SemaType.cpp
index 426cd0aa91c01..e2063869d4042 100644
--- a/clang/lib/Sema/SemaType.cpp
+++ b/clang/lib/Sema/SemaType.cpp
@@ -7623,6 +7623,7 @@ handleNonBlockingNonAllocatingTypeAttr(TypeProcessingState &TPState,
FunctionEffectSet FX(EPI.FunctionEffects);
FunctionEffectSet::Conflicts Errs;
bool Success = FX.insert(NewEC, Errs);
+ (void)Success;
assert(Success && "effect conflicts should have been diagnosed above");
EPI.FunctionEffects = FunctionEffectsRef(FX);
>From 58a1dc02dc578571f648631f1519195e2d018630 Mon Sep 17 00:00:00 2001
From: Doug Wyatt <dwyatt at apple.com>
Date: Fri, 12 Jul 2024 08:51:15 -0700
Subject: [PATCH 2/2] Bypass Sema's function effect checks in the case where
the ASTContext has never seen any function effects.
---
clang/include/clang/AST/ASTContext.h | 5 ++
clang/lib/AST/ASTContext.cpp | 2 +
clang/lib/Sema/Sema.cpp | 2 +-
clang/lib/Sema/SemaDecl.cpp | 84 ++++++++++++++--------------
clang/lib/Sema/SemaDeclCXX.cpp | 62 ++++++++++----------
clang/lib/Sema/SemaOverload.cpp | 2 +-
6 files changed, 84 insertions(+), 73 deletions(-)
diff --git a/clang/include/clang/AST/ASTContext.h b/clang/include/clang/AST/ASTContext.h
index 7aa1357b9aad0..3112675d8778e 100644
--- a/clang/include/clang/AST/ASTContext.h
+++ b/clang/include/clang/AST/ASTContext.h
@@ -642,6 +642,9 @@ class ASTContext : public RefCountedBase<ASTContext> {
/// address spaces (e.g. OpenCL/CUDA)
bool AddrSpaceMapMangling;
+ /// For performance, track whether any function effects are in use.
+ mutable bool AnyFunctionEffects = false;
+
const TargetInfo *Target = nullptr;
const TargetInfo *AuxTarget = nullptr;
clang::PrintingPolicy PrintingPolicy;
@@ -2896,6 +2899,8 @@ class ASTContext : public RefCountedBase<ASTContext> {
return AddrSpaceMapMangling || isTargetAddressSpace(AS);
}
+ bool hasAnyFunctionEffects() const { return AnyFunctionEffects; }
+
// Merges two exception specifications, such that the resulting
// exception spec is the union of both. For example, if either
// of them can throw something, the result can throw it as well.
diff --git a/clang/lib/AST/ASTContext.cpp b/clang/lib/AST/ASTContext.cpp
index b5519b3fc25a3..6c3264000a83f 100644
--- a/clang/lib/AST/ASTContext.cpp
+++ b/clang/lib/AST/ASTContext.cpp
@@ -4617,6 +4617,8 @@ QualType ASTContext::getFunctionTypeInternal(
Types.push_back(FTP);
if (!Unique)
FunctionProtoTypes.InsertNode(FTP, InsertPos);
+ if (!EPI.FunctionEffects.empty())
+ AnyFunctionEffects = true;
return QualType(FTP, 0);
}
diff --git a/clang/lib/Sema/Sema.cpp b/clang/lib/Sema/Sema.cpp
index 3f8f2f027172d..80980f35177ef 100644
--- a/clang/lib/Sema/Sema.cpp
+++ b/clang/lib/Sema/Sema.cpp
@@ -728,7 +728,7 @@ ExprResult Sema::ImpCastExprToType(Expr *E, QualType Ty,
diagnoseNullableToNonnullConversion(Ty, E->getType(), E->getBeginLoc());
diagnoseZeroToNullptrConversion(Kind, E);
- if (!isCast(CCK) && Kind != CK_NullToPointer &&
+ if (Context.hasAnyFunctionEffects() && !isCast(CCK) && Kind != CK_NullToPointer &&
Kind != CK_NullToMemberPointer)
diagnoseFunctionEffectConversion(Ty, E->getType(), E->getBeginLoc());
diff --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp
index 029ccf944c513..1f1271a6e06d0 100644
--- a/clang/lib/Sema/SemaDecl.cpp
+++ b/clang/lib/Sema/SemaDecl.cpp
@@ -3911,48 +3911,50 @@ bool Sema::MergeFunctionDecl(FunctionDecl *New, NamedDecl *&OldD, Scope *S,
return true;
}
- const auto OldFX = Old->getFunctionEffects();
- const auto NewFX = New->getFunctionEffects();
QualType OldQTypeForComparison = OldQType;
- if (OldFX != NewFX) {
- const auto Diffs = FunctionEffectDifferences(OldFX, NewFX);
- for (const auto &Diff : Diffs) {
- if (Diff.shouldDiagnoseRedeclaration(*Old, OldFX, *New, NewFX)) {
- Diag(New->getLocation(),
- diag::warn_mismatched_func_effect_redeclaration)
- << Diff.effectName();
- Diag(Old->getLocation(), diag::note_previous_declaration);
- }
- }
- // Following a warning, we could skip merging effects from the previous
- // declaration, but that would trigger an additional "conflicting types"
- // error.
- if (const auto *NewFPT = NewQType->getAs<FunctionProtoType>()) {
- FunctionEffectSet::Conflicts MergeErrs;
- FunctionEffectSet MergedFX =
- FunctionEffectSet::getUnion(OldFX, NewFX, MergeErrs);
- if (!MergeErrs.empty())
- diagnoseFunctionEffectMergeConflicts(MergeErrs, New->getLocation(),
- Old->getLocation());
-
- FunctionProtoType::ExtProtoInfo EPI = NewFPT->getExtProtoInfo();
- EPI.FunctionEffects = FunctionEffectsRef(MergedFX);
- QualType ModQT = Context.getFunctionType(NewFPT->getReturnType(),
- NewFPT->getParamTypes(), EPI);
-
- New->setType(ModQT);
- NewQType = New->getType();
-
- // Revise OldQTForComparison to include the merged effects,
- // so as not to fail due to differences later.
- if (const auto *OldFPT = OldQType->getAs<FunctionProtoType>()) {
- EPI = OldFPT->getExtProtoInfo();
- EPI.FunctionEffects = FunctionEffectsRef(MergedFX);
- OldQTypeForComparison = Context.getFunctionType(
- OldFPT->getReturnType(), OldFPT->getParamTypes(), EPI);
- }
- }
- }
+ if (Context.hasAnyFunctionEffects()) {
+ const auto OldFX = Old->getFunctionEffects();
+ const auto NewFX = New->getFunctionEffects();
+ if (OldFX != NewFX) {
+ const auto Diffs = FunctionEffectDifferences(OldFX, NewFX);
+ for (const auto &Diff : Diffs) {
+ if (Diff.shouldDiagnoseRedeclaration(*Old, OldFX, *New, NewFX)) {
+ Diag(New->getLocation(),
+ diag::warn_mismatched_func_effect_redeclaration)
+ << Diff.effectName();
+ Diag(Old->getLocation(), diag::note_previous_declaration);
+ }
+ }
+ // Following a warning, we could skip merging effects from the previous
+ // declaration, but that would trigger an additional "conflicting types"
+ // error.
+ if (const auto *NewFPT = NewQType->getAs<FunctionProtoType>()) {
+ FunctionEffectSet::Conflicts MergeErrs;
+ FunctionEffectSet MergedFX =
+ FunctionEffectSet::getUnion(OldFX, NewFX, MergeErrs);
+ if (!MergeErrs.empty())
+ diagnoseFunctionEffectMergeConflicts(MergeErrs, New->getLocation(),
+ Old->getLocation());
+
+ FunctionProtoType::ExtProtoInfo EPI = NewFPT->getExtProtoInfo();
+ EPI.FunctionEffects = FunctionEffectsRef(MergedFX);
+ QualType ModQT = Context.getFunctionType(NewFPT->getReturnType(),
+ NewFPT->getParamTypes(), EPI);
+
+ New->setType(ModQT);
+ NewQType = New->getType();
+
+ // Revise OldQTForComparison to include the merged effects,
+ // so as not to fail due to differences later.
+ if (const auto *OldFPT = OldQType->getAs<FunctionProtoType>()) {
+ EPI = OldFPT->getExtProtoInfo();
+ EPI.FunctionEffects = FunctionEffectsRef(MergedFX);
+ OldQTypeForComparison = Context.getFunctionType(
+ OldFPT->getReturnType(), OldFPT->getParamTypes(), EPI);
+ }
+ }
+ }
+ }
if (getLangOpts().CPlusPlus) {
OldQType = Context.getCanonicalType(Old->getType());
diff --git a/clang/lib/Sema/SemaDeclCXX.cpp b/clang/lib/Sema/SemaDeclCXX.cpp
index 9b220103247dd..756c5848e81e5 100644
--- a/clang/lib/Sema/SemaDeclCXX.cpp
+++ b/clang/lib/Sema/SemaDeclCXX.cpp
@@ -18328,38 +18328,40 @@ bool Sema::CheckOverridingFunctionAttributes(CXXMethodDecl *New,
}
// Virtual overrides: check for matching effects.
- const auto OldFX = Old->getFunctionEffects();
- const auto NewFXOrig = New->getFunctionEffects();
-
- if (OldFX != NewFXOrig) {
- FunctionEffectSet NewFX(NewFXOrig);
- const auto Diffs = FunctionEffectDifferences(OldFX, NewFX);
- FunctionEffectSet::Conflicts Errs;
- for (const auto &Diff : Diffs) {
- switch (Diff.shouldDiagnoseMethodOverride(*Old, OldFX, *New, NewFX)) {
- case FunctionEffectDiff::OverrideResult::NoAction:
- break;
- case FunctionEffectDiff::OverrideResult::Warn:
- Diag(New->getLocation(), diag::warn_mismatched_func_effect_override)
- << Diff.effectName();
- Diag(Old->getLocation(), diag::note_overridden_virtual_function)
- << Old->getReturnTypeSourceRange();
- break;
- case FunctionEffectDiff::OverrideResult::Merge: {
- NewFX.insert(Diff.Old, Errs);
- const auto *NewFT = New->getType()->castAs<FunctionProtoType>();
- FunctionProtoType::ExtProtoInfo EPI = NewFT->getExtProtoInfo();
- EPI.FunctionEffects = FunctionEffectsRef(NewFX);
- QualType ModQT = Context.getFunctionType(NewFT->getReturnType(),
- NewFT->getParamTypes(), EPI);
- New->setType(ModQT);
- break;
- }
+ if (Context.hasAnyFunctionEffects()) {
+ const auto OldFX = Old->getFunctionEffects();
+ const auto NewFXOrig = New->getFunctionEffects();
+
+ if (OldFX != NewFXOrig) {
+ FunctionEffectSet NewFX(NewFXOrig);
+ const auto Diffs = FunctionEffectDifferences(OldFX, NewFX);
+ FunctionEffectSet::Conflicts Errs;
+ for (const auto &Diff : Diffs) {
+ switch (Diff.shouldDiagnoseMethodOverride(*Old, OldFX, *New, NewFX)) {
+ case FunctionEffectDiff::OverrideResult::NoAction:
+ break;
+ case FunctionEffectDiff::OverrideResult::Warn:
+ Diag(New->getLocation(), diag::warn_mismatched_func_effect_override)
+ << Diff.effectName();
+ Diag(Old->getLocation(), diag::note_overridden_virtual_function)
+ << Old->getReturnTypeSourceRange();
+ break;
+ case FunctionEffectDiff::OverrideResult::Merge: {
+ NewFX.insert(Diff.Old, Errs);
+ const auto *NewFT = New->getType()->castAs<FunctionProtoType>();
+ FunctionProtoType::ExtProtoInfo EPI = NewFT->getExtProtoInfo();
+ EPI.FunctionEffects = FunctionEffectsRef(NewFX);
+ QualType ModQT = Context.getFunctionType(NewFT->getReturnType(),
+ NewFT->getParamTypes(), EPI);
+ New->setType(ModQT);
+ break;
+ }
+ }
}
+ if (!Errs.empty())
+ diagnoseFunctionEffectMergeConflicts(Errs, New->getLocation(),
+ Old->getLocation());
}
- if (!Errs.empty())
- diagnoseFunctionEffectMergeConflicts(Errs, New->getLocation(),
- Old->getLocation());
}
CallingConv NewCC = NewFT->getCallConv(), OldCC = OldFT->getCallConv();
diff --git a/clang/lib/Sema/SemaOverload.cpp b/clang/lib/Sema/SemaOverload.cpp
index db77e5cfc1957..56e5cd89d600d 100644
--- a/clang/lib/Sema/SemaOverload.cpp
+++ b/clang/lib/Sema/SemaOverload.cpp
@@ -1882,7 +1882,7 @@ bool Sema::IsFunctionConversion(QualType FromType, QualType ToType,
// we need to not alter FromFn, or else even an innocuous cast
// like dropping effects will fail. In C++ however we do want to
// alter FromFn (because of the way PerformImplicitConversion works).
- if (getLangOpts().CPlusPlus) {
+ if (Context.hasAnyFunctionEffects() && getLangOpts().CPlusPlus) {
FromFPT = cast<FunctionProtoType>(FromFn); // in case FromFn changed above
// Transparently add/drop effects; here we are concerned with
More information about the cfe-commits
mailing list