[clang][bytecode][NFC] Cache the BuiltinID in Function

[tbaederr]

FunctionDecl::getBuiltinID() is surprisingly slow and we tend to call it quite a bit, especially when interpreting builtin functions. Caching the BuiltinID here reduces the time I need to compile the floating_comparison namespace from builtin-functions.cpp from 7.2s to 6.3s locally.

[llvmbot]

@llvm/pr-subscribers-clang

Author: Timm Baeder (tbaederr)

Changes

FunctionDecl::getBuiltinID() is surprisingly slow and we tend to call it quite a bit, especially when interpreting builtin functions. Caching the BuiltinID here reduces the time I need to compile the floating_comparison namespace from builtin-functions.cpp from 7.2s to 6.3s locally.

---

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

3 Files Affected:

• (modified) clang/lib/AST/ByteCode/ByteCodeEmitter.cpp (+2-16)
• (modified) clang/lib/AST/ByteCode/Function.cpp (+17-3)
• (modified) clang/lib/AST/ByteCode/Function.h (+5-9)

diff --git a/clang/lib/AST/ByteCode/ByteCodeEmitter.cpp b/clang/lib/AST/ByteCode/ByteCodeEmitter.cpp
index 35ae1547939fdd..b8778f6027894c 100644
--- a/clang/lib/AST/ByteCode/ByteCodeEmitter.cpp
+++ b/clang/lib/AST/ByteCode/ByteCodeEmitter.cpp
@@ -21,17 +21,6 @@
 using namespace clang;
 using namespace clang::interp;
 
-/// Unevaluated builtins don't get their arguments put on the stack
-/// automatically. They instead operate on the AST of their Call
-/// Expression.
-/// Similar information is available via ASTContext::BuiltinInfo,
-/// but that is not correct for our use cases.
-static bool isUnevaluatedBuiltin(unsigned BuiltinID) {
-  return BuiltinID == Builtin::BI__builtin_classify_type ||
-         BuiltinID == Builtin::BI__builtin_os_log_format_buffer_size ||
-         BuiltinID == Builtin::BI__builtin_constant_p;
-}
-
 Function *ByteCodeEmitter::compileFunc(const FunctionDecl *FuncDecl) {
 
   // Manually created functions that haven't been assigned proper
@@ -147,14 +136,11 @@ Function *ByteCodeEmitter::compileFunc(const FunctionDecl *FuncDecl) {
   // Create a handle over the emitted code.
   Function *Func = P.getFunction(FuncDecl);
   if (!Func) {
-    bool IsUnevaluatedBuiltin = false;
-    if (unsigned BI = FuncDecl->getBuiltinID())
-      IsUnevaluatedBuiltin = isUnevaluatedBuiltin(BI);
-
+    unsigned BuiltinID = FuncDecl->getBuiltinID();
     Func =
         P.createFunction(FuncDecl, ParamOffset, std::move(ParamTypes),
                          std::move(ParamDescriptors), std::move(ParamOffsets),
-                         HasThisPointer, HasRVO, IsUnevaluatedBuiltin);
+                         HasThisPointer, HasRVO, BuiltinID);
   }
 
   assert(Func);
diff --git a/clang/lib/AST/ByteCode/Function.cpp b/clang/lib/AST/ByteCode/Function.cpp
index e3fab3f6720b41..25da6ae1bc7b61 100644
--- a/clang/lib/AST/ByteCode/Function.cpp
+++ b/clang/lib/AST/ByteCode/Function.cpp
@@ -20,11 +20,10 @@ Function::Function(Program &P, FunctionDeclTy Source, unsigned ArgSize,
                    llvm::SmallVectorImpl<PrimType> &&ParamTypes,
                    llvm::DenseMap<unsigned, ParamDescriptor> &&Params,
                    llvm::SmallVectorImpl<unsigned> &&ParamOffsets,
-                   bool HasThisPointer, bool HasRVO, bool UnevaluatedBuiltin)
+                   bool HasThisPointer, bool HasRVO, unsigned BuiltinID)
     : P(P), Source(Source), ArgSize(ArgSize), ParamTypes(std::move(ParamTypes)),
       Params(std::move(Params)), ParamOffsets(std::move(ParamOffsets)),
-      HasThisPointer(HasThisPointer), HasRVO(HasRVO),
-      IsUnevaluatedBuiltin(UnevaluatedBuiltin) {
+      HasThisPointer(HasThisPointer), HasRVO(HasRVO), BuiltinID(BuiltinID) {
   if (const auto *F = Source.dyn_cast<const FunctionDecl *>())
     Variadic = F->isVariadic();
 }
@@ -53,3 +52,18 @@ bool Function::isVirtual() const {
     return M->isVirtual();
   return false;
 }
+
+/// Unevaluated builtins don't get their arguments put on the stack
+/// automatically. They instead operate on the AST of their Call
+/// Expression.
+/// Similar information is available via ASTContext::BuiltinInfo,
+/// but that is not correct for our use cases.
+static bool isUnevaluatedBuiltin(unsigned BuiltinID) {
+  return BuiltinID == Builtin::BI__builtin_classify_type ||
+         BuiltinID == Builtin::BI__builtin_os_log_format_buffer_size ||
+         BuiltinID == Builtin::BI__builtin_constant_p;
+}
+
+bool Function::isUnevaluatedBuiltin() const {
+  return ::isUnevaluatedBuiltin(BuiltinID);
+}
diff --git a/clang/lib/AST/ByteCode/Function.h b/clang/lib/AST/ByteCode/Function.h
index f254db20d4f594..b21fa8497130ea 100644
--- a/clang/lib/AST/ByteCode/Function.h
+++ b/clang/lib/AST/ByteCode/Function.h
@@ -193,15 +193,11 @@ class Function final {
 
   bool isVariadic() const { return Variadic; }
 
-  unsigned getBuiltinID() const {
-    return Source.get<const FunctionDecl *>()->getBuiltinID();
-  }
+  unsigned getBuiltinID() const { return BuiltinID; }
 
-  bool isBuiltin() const {
-    return Source.get<const FunctionDecl *>()->getBuiltinID() != 0;
-  }
+  bool isBuiltin() const { return getBuiltinID() != 0; }
 
-  bool isUnevaluatedBuiltin() const { return IsUnevaluatedBuiltin; }
+  bool isUnevaluatedBuiltin() const;
 
   unsigned getNumParams() const { return ParamTypes.size(); }
 
@@ -232,7 +228,7 @@ class Function final {
            llvm::SmallVectorImpl<PrimType> &&ParamTypes,
            llvm::DenseMap<unsigned, ParamDescriptor> &&Params,
            llvm::SmallVectorImpl<unsigned> &&ParamOffsets, bool HasThisPointer,
-           bool HasRVO, bool UnevaluatedBuiltin);
+           bool HasRVO, unsigned BuiltinID);
 
   /// Sets the code of a function.
   void setCode(unsigned NewFrameSize, std::vector<std::byte> &&NewCode,
@@ -289,7 +285,7 @@ class Function final {
   bool HasBody = false;
   bool Defined = false;
   bool Variadic = false;
-  bool IsUnevaluatedBuiltin = false;
+  unsigned BuiltinID = 0;
 
 public:
   /// Dumps the disassembled bytecode to \c llvm::errs().