Performance optimizations for function effects (nonblocking attribute etc.) #96844

dougsonos · 2024-06-27T04:02:17Z

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

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

llvmbot · 2024-06-27T04:02:49Z

@llvm/pr-subscribers-clang

Author: Doug Wyatt (dougsonos)

Changes

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

Full diff: https://github.com/llvm/llvm-project/pull/96844.diff

4 Files Affected:

(modified) clang/include/clang/AST/Type.h (+22-8)
(modified) clang/lib/AST/ASTContext.cpp (+6-5)
(modified) clang/lib/AST/Type.cpp (+12-25)
(modified) clang/lib/Sema/SemaType.cpp (+1)

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);

dougsonos · 2024-06-27T04:03:55Z

This reduces the compile-time regression introduced by f03cb00
[Clang] Introduce nonblocking/nonallocating attributes (#84983). This branch is being monitored by the compile-time tracker: http://llvm-compile-time-tracker.com/index.php

More optimizations may be possible, but I'm about to go away from computers (until July 9) and this is the best I've got so far.

@Sirraide
@AaronBallman
@nikic

clang/include/clang/AST/Type.h

clang/lib/AST/Type.cpp

dougsonos · 2024-06-27T04:09:43Z

clang/lib/AST/Type.cpp

+      (epi.AArch64SMEAttributes << 1) | epi.HasTrailingReturn);
+
+  for (unsigned Idx = 0; Idx != EffectCount; ++Idx) {
+		ID.AddInteger(epi.FunctionEffects.Effects[Idx].toOpaqueInt32());


Oops, I forgot to run clang-format.

dougsonos · 2024-06-27T04:10:15Z

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;


From the previous branch - was generating a warning when assertions are disabled.

A better way to do this would be [[maybe_unused]] on the declaration instead of a cast to void.

clang/include/clang/AST/Type.h

tbaederr · 2024-06-27T04:25:21Z

I guess you meant to link to https://llvm-compile-time-tracker.com/compare.php?from=f03cb005eb4ba3c6fb645aca2228e907db8cd452&to=038b39d3235c6c8151127c34d34f498dd298273c&stat=instructions:u directly?

dougsonos · 2024-06-27T04:32:32Z

I guess you meant to link to https://llvm-compile-time-tracker.com/compare.php?from=f03cb005eb4ba3c6fb645aca2228e907db8cd452&to=038b39d3235c6c8151127c34d34f498dd298273c&stat=instructions:u directly?

Thanks!

AaronBallman

Thank you for working on this! The changes LGTM aside from nits.

AaronBallman · 2024-06-27T14:47:35Z

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;


A better way to do this would be [[maybe_unused]] on the declaration instead of a cast to void.

… has never seen any function effects.

dougsonos · 2024-07-12T17:43:03Z

Added a further tweak: add a bool to ASTContext to track whether any FunctionProtoType's with effects have been created, so that Sema can short-circuit checks involving effects when it knows no one has any. This seems to have the desired effect of only paying for the feature when it's in use.

Sirraide · 2024-07-15T16:05:16Z

This seems to have the desired effect of only paying for the feature when it's in use.

It does seem to have helped a bit, yeah.

dougsonos · 2024-07-17T17:24:48Z

It would be great if we could merge this soon, please; I'd like to start another PR for the 2nd-pass effects analysis and it has a dependency or two on these changes.

ping @nikic

nikic · 2024-07-17T19:12:51Z

Thanks for following up on this!

For the record, this was the final result: https://llvm-compile-time-tracker.com/compare.php?from=18cdfa72e046a40deeee4372ee98602fd1a65a94&to=0bb68b55715487447ffceaa1ab59f7a0bc8c7979&stat=instructions:u

… etc.) (llvm#96844) - Put new FunctionProtoType trailing objects last. - Inline FunctionEffectsRef::get() - Manually inline FunctionEffectsRef::Profile(). --------- Co-authored-by: Doug Wyatt <[email protected]>

… etc.) (#96844) Summary: - Put new FunctionProtoType trailing objects last. - Inline FunctionEffectsRef::get() - Manually inline FunctionEffectsRef::Profile(). --------- Co-authored-by: Doug Wyatt <[email protected]> Test Plan: Reviewers: Subscribers: Tasks: Tags: Differential Revision: https://phabricator.intern.facebook.com/D60250936

Performance optimizations for function effects:

038b39d

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

llvmbot added clang Clang issues not falling into any other category clang:frontend Language frontend issues, e.g. anything involving "Sema" labels Jun 27, 2024

dougsonos commented Jun 27, 2024

View reviewed changes

clang/include/clang/AST/Type.h Show resolved Hide resolved

dougsonos commented Jun 27, 2024

View reviewed changes

clang/lib/AST/Type.cpp Outdated Show resolved Hide resolved

dougsonos commented Jun 27, 2024

View reviewed changes

clang/include/clang/AST/Type.h Show resolved Hide resolved

dougsonos commented Jun 27, 2024

View reviewed changes

clang/include/clang/AST/Type.h Show resolved Hide resolved

AaronBallman requested review from nikic, AaronBallman and Sirraide June 27, 2024 14:46

AaronBallman approved these changes Jun 27, 2024

View reviewed changes

Doug Wyatt added 3 commits July 12, 2024 08:51

Bypass Sema's function effect checks in the case where the ASTContext…

58a1dc0

… has never seen any function effects.

Review feedback: [[maybe_unused]]

cc689ac

clang-format

4b6d61a

Sirraide approved these changes Jul 15, 2024

View reviewed changes

AaronBallman merged commit 0bb68b5 into llvm:main Jul 17, 2024
7 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Performance optimizations for function effects (nonblocking attribute etc.) #96844

Performance optimizations for function effects (nonblocking attribute etc.) #96844

dougsonos commented Jun 27, 2024

llvmbot commented Jun 27, 2024

dougsonos commented Jun 27, 2024 •

edited

Loading

dougsonos Jun 27, 2024

dougsonos Jun 27, 2024

AaronBallman Jun 27, 2024

tbaederr commented Jun 27, 2024

dougsonos commented Jun 27, 2024

AaronBallman left a comment

AaronBallman Jun 27, 2024

dougsonos commented Jul 12, 2024

Sirraide commented Jul 15, 2024

dougsonos commented Jul 17, 2024 •

edited

Loading

nikic commented Jul 17, 2024

Performance optimizations for function effects (nonblocking attribute etc.) #96844

Performance optimizations for function effects (nonblocking attribute etc.) #96844

Conversation

dougsonos commented Jun 27, 2024

llvmbot commented Jun 27, 2024

dougsonos commented Jun 27, 2024 • edited Loading

dougsonos Jun 27, 2024

Choose a reason for hiding this comment

dougsonos Jun 27, 2024

Choose a reason for hiding this comment

AaronBallman Jun 27, 2024

Choose a reason for hiding this comment

tbaederr commented Jun 27, 2024

dougsonos commented Jun 27, 2024

AaronBallman left a comment

Choose a reason for hiding this comment

AaronBallman Jun 27, 2024

Choose a reason for hiding this comment

dougsonos commented Jul 12, 2024

Sirraide commented Jul 15, 2024

dougsonos commented Jul 17, 2024 • edited Loading

nikic commented Jul 17, 2024

dougsonos commented Jun 27, 2024 •

edited

Loading

dougsonos commented Jul 17, 2024 •

edited

Loading