Skip to content

Commit

Permalink
Performance optimizations for function effects (nonblocking attribute…
Browse files Browse the repository at this point in the history 
… etc.) (llvm#96844)

- Put new FunctionProtoType trailing objects last.
- Inline FunctionEffectsRef::get()
- Manually inline FunctionEffectsRef::Profile().

---------

Co-authored-by: Doug Wyatt <[email protected]>
  • Loading branch information
@dougsonos @sgundapa
2 people authored and sgundapa committed Jul 23, 2024
1 parent ccf9a39 commit a74bca0
Show file tree
Hide file tree
Showing 8 changed files with 119 additions and 108 deletions.
5 changes: 5 additions & 0 deletions clang/include/clang/AST/ASTContext.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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.
Expand Down
32 changes: 23 additions & 9 deletions clang/include/clang/AST/Type.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,6 @@ class TemplateArgument;
class TemplateArgumentListInfo;
class TemplateArgumentLoc;
class TemplateTypeParmDecl;
template <typename> class TreeTransform;
class TypedefNameDecl;
class UnresolvedUsingTypenameDecl;
class UsingShadowDecl;
Expand Down Expand Up @@ -4901,7 +4900,6 @@ class FunctionEffectsRef {
return !(LHS == RHS);
}

void Profile(llvm::FoldingSetNodeID &ID) const;
void dump(llvm::raw_ostream &OS) const;
};

Expand Down Expand Up @@ -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;

Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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();
}
Expand Down Expand Up @@ -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
12 changes: 7 additions & 5 deletions clang/lib/AST/ASTContext.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4896,21 +4896,23 @@ 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;
new (FTP) FunctionProtoType(ResultTy, ArgArray, Canonical, newEPI);
Types.push_back(FTP);
if (!Unique)
FunctionProtoTypes.InsertNode(FTP, InsertPos);
if (!EPI.FunctionEffects.empty())
AnyFunctionEffects = true;
return QualType(FTP, 0);
}

Expand Down
36 changes: 11 additions & 25 deletions clang/lib/AST/Type.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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) {
Expand Down
4 changes: 2 additions & 2 deletions clang/lib/Sema/Sema.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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());
Expand Down
74 changes: 38 additions & 36 deletions clang/lib/Sema/SemaDecl.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 differences 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 differences later.
if (const auto *OldFPT = OldQType->getAs<FunctionProtoType>()) {
EPI = OldFPT->getExtProtoInfo();
EPI.FunctionEffects = FunctionEffectsRef(MergedFX);
OldQTypeForComparison = Context.getFunctionType(
OldFPT->getReturnType(), OldFPT->getParamTypes(), EPI);
}
}
}
}
Expand Down
62 changes: 32 additions & 30 deletions clang/lib/Sema/SemaDeclCXX.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
2 changes: 1 addition & 1 deletion clang/lib/Sema/SemaOverload.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit a74bca0

Please sign in to comment.