[clang] 0bb68b5 - Performance optimizations for function effects (nonblocking attribute etc.) (#96844)
via cfe-commits
cfe-commits at lists.llvm.org
Wed Jul 17 10:36:40 PDT 2024
Author: Doug Wyatt
Date: 2024-07-17T13:36:36-04:00
New Revision: 0bb68b55715487447ffceaa1ab59f7a0bc8c7979
URL: https://github.com/llvm/llvm-project/commit/0bb68b55715487447ffceaa1ab59f7a0bc8c7979
DIFF: https://github.com/llvm/llvm-project/commit/0bb68b55715487447ffceaa1ab59f7a0bc8c7979.diff
LOG: Performance optimizations for function effects (nonblocking attribute etc.) (#96844)
- Put new FunctionProtoType trailing objects last.
- Inline FunctionEffectsRef::get()
- Manually inline FunctionEffectsRef::Profile().
---------
Co-authored-by: Doug Wyatt <dwyatt at apple.com>
Added:
Modified:
clang/include/clang/AST/ASTContext.h
clang/include/clang/AST/Type.h
clang/lib/AST/ASTContext.cpp
clang/lib/AST/Type.cpp
clang/lib/Sema/Sema.cpp
clang/lib/Sema/SemaDecl.cpp
clang/lib/Sema/SemaDeclCXX.cpp
clang/lib/Sema/SemaOverload.cpp
Removed:
################################################################################
diff --git a/clang/include/clang/AST/ASTContext.h b/clang/include/clang/AST/ASTContext.h
index 13aa203de32ba..608bd90fcc3ff 100644
--- a/clang/include/clang/AST/ASTContext.h
+++ b/clang/include/clang/AST/ASTContext.h
@@ -643,6 +643,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;
@@ -2909,6 +2912,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/include/clang/AST/Type.h b/clang/include/clang/AST/Type.h
index 4c9ba37fe1e3a..25defea58c2dc 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;
@@ -4901,7 +4900,6 @@ class FunctionEffectsRef {
return !(LHS == RHS);
}
- void Profile(llvm::FoldingSetNodeID &ID) const;
void dump(llvm::raw_ostream &OS) const;
};
@@ -4971,8 +4969,8 @@ class FunctionProtoType final
FunctionProtoType, QualType, SourceLocation,
FunctionType::FunctionTypeExtraBitfields,
FunctionType::FunctionTypeArmAttributes, FunctionType::ExceptionType,
- Expr *, FunctionDecl *, FunctionType::ExtParameterInfo,
- FunctionEffect, EffectConditionExpr, Qualifiers> {
+ Expr *, FunctionDecl *, FunctionType::ExtParameterInfo, Qualifiers,
+ FunctionEffect, EffectConditionExpr> {
friend class ASTContext; // ASTContext creates these.
friend TrailingObjects;
@@ -5003,21 +5001,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
@@ -5134,6 +5132,10 @@ class FunctionProtoType final
return hasExtParameterInfos() ? getNumParams() : 0;
}
+ unsigned numTrailingObjects(OverloadToken<Qualifiers>) const {
+ return hasExtQualifiers() ? 1 : 0;
+ }
+
unsigned numTrailingObjects(OverloadToken<FunctionEffect>) const {
return getNumFunctionEffects();
}
@@ -8616,6 +8618,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 f4aa1387974aa..a8e599f7ebe04 100644
--- a/clang/lib/AST/ASTContext.cpp
+++ b/clang/lib/AST/ASTContext.cpp
@@ -4896,14 +4896,14 @@ 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;
@@ -4911,6 +4911,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/AST/Type.cpp b/clang/lib/AST/Type.cpp
index 5bf1f3dbdbd4b..fdaab8e434593 100644
--- a/clang/lib/AST/Type.cpp
+++ b/clang/lib/AST/Type.cpp
@@ -3798,9 +3798,18 @@ 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,
@@ -5181,17 +5190,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();
@@ -5313,18 +5311,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/Sema.cpp b/clang/lib/Sema/Sema.cpp
index d6228718d53ae..46417964f0896 100644
--- a/clang/lib/Sema/Sema.cpp
+++ b/clang/lib/Sema/Sema.cpp
@@ -718,8 +718,8 @@ ExprResult Sema::ImpCastExprToType(Expr *E, QualType Ty,
diagnoseNullableToNonnullConversion(Ty, E->getType(), E->getBeginLoc());
diagnoseZeroToNullptrConversion(Kind, E);
- if (!isCast(CCK) && Kind != CK_NullToPointer &&
- Kind != CK_NullToMemberPointer)
+ if (Context.hasAnyFunctionEffects() && !isCast(CCK) &&
+ Kind != CK_NullToPointer && Kind != CK_NullToMemberPointer)
diagnoseFunctionEffectConversion(Ty, E->getType(), E->getBeginLoc());
QualType ExprTy = Context.getCanonicalType(E->getType());
diff --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp
index 1f2fde12c9d24..a3dd5ede9116a 100644
--- a/clang/lib/Sema/SemaDecl.cpp
+++ b/clang/lib/Sema/SemaDecl.cpp
@@ -3813,45 +3813,47 @@ 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);
+ 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
diff erences later.
- if (const auto *OldFPT = OldQType->getAs<FunctionProtoType>()) {
- EPI = OldFPT->getExtProtoInfo();
+ // 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);
- OldQTypeForComparison = Context.getFunctionType(
- OldFPT->getReturnType(), OldFPT->getParamTypes(), EPI);
+ 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
diff erences later.
+ if (const auto *OldFPT = OldQType->getAs<FunctionProtoType>()) {
+ EPI = OldFPT->getExtProtoInfo();
+ EPI.FunctionEffects = FunctionEffectsRef(MergedFX);
+ OldQTypeForComparison = Context.getFunctionType(
+ OldFPT->getReturnType(), OldFPT->getParamTypes(), EPI);
+ }
}
}
}
diff --git a/clang/lib/Sema/SemaDeclCXX.cpp b/clang/lib/Sema/SemaDeclCXX.cpp
index 2bfb103e8953d..04b8d88cae217 100644
--- a/clang/lib/Sema/SemaDeclCXX.cpp
+++ b/clang/lib/Sema/SemaDeclCXX.cpp
@@ -18100,38 +18100,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 074062ebbb594..472e7ae5d1d3f 100644
--- a/clang/lib/Sema/SemaOverload.cpp
+++ b/clang/lib/Sema/SemaOverload.cpp
@@ -1863,7 +1863,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