Skip to content

[CIR] Upstream EHScopeStack memory allocator #152215

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Aug 7, 2025

Conversation

andykaylor
Copy link
Contributor

When the cleanup handling code was initially upstreamed, a SmallVector was used to simplify the handling of the stack of cleanup objects. However, that mechanism won't scale well enough for the rate at which cleanup handlers are going to be pushed and popped while compiling a large program. This change introduces the custom memory allocator which is used in classic codegen and the CIR incubator.

Thiis does not otherwise change the cleanup handling implementation and many parts of the infrastructure are still missing.

This is not intended to have any observable effect on the generated CIR, but it does change the internal implementation significantly, so it's not exactly an NFC change. The functionality is covered by existing tests.

@llvmbot llvmbot added clang Clang issues not falling into any other category ClangIR Anything related to the ClangIR project labels Aug 5, 2025
@llvmbot
Copy link
Member

llvmbot commented Aug 5, 2025

@llvm/pr-subscribers-clangir

@llvm/pr-subscribers-clang

Author: Andy Kaylor (andykaylor)

Changes

When the cleanup handling code was initially upstreamed, a SmallVector was used to simplify the handling of the stack of cleanup objects. However, that mechanism won't scale well enough for the rate at which cleanup handlers are going to be pushed and popped while compiling a large program. This change introduces the custom memory allocator which is used in classic codegen and the CIR incubator.

Thiis does not otherwise change the cleanup handling implementation and many parts of the infrastructure are still missing.

This is not intended to have any observable effect on the generated CIR, but it does change the internal implementation significantly, so it's not exactly an NFC change. The functionality is covered by existing tests.


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

8 Files Affected:

  • (modified) clang/include/clang/CIR/MissingFeatures.h (+1)
  • (modified) clang/lib/CIR/CodeGen/CIRGenCleanup.cpp (+65-8)
  • (added) clang/lib/CIR/CodeGen/CIRGenCleanup.h (+43)
  • (modified) clang/lib/CIR/CodeGen/CIRGenDecl.cpp (+6)
  • (modified) clang/lib/CIR/CodeGen/CIRGenFunction.cpp (+4-4)
  • (modified) clang/lib/CIR/CodeGen/CIRGenFunction.h (+8-4)
  • (modified) clang/lib/CIR/CodeGen/CIRGenStmt.cpp (+1-1)
  • (modified) clang/lib/CIR/CodeGen/EHScopeStack.h (+94-9)
diff --git a/clang/include/clang/CIR/MissingFeatures.h b/clang/include/clang/CIR/MissingFeatures.h
index 27dd181f2fb37..492dfbb3b0aab 100644
--- a/clang/include/clang/CIR/MissingFeatures.h
+++ b/clang/include/clang/CIR/MissingFeatures.h
@@ -199,6 +199,7 @@ struct MissingFeatures {
   static bool dataLayoutTypeAllocSize() { return false; }
   static bool deferredCXXGlobalInit() { return false; }
   static bool ehCleanupFlags() { return false; }
+  static bool ehCleanupScope() { return false; }
   static bool ehstackBranches() { return false; }
   static bool emitCheckedInBoundsGEP() { return false; }
   static bool emitCondLikelihoodViaExpectIntrinsic() { return false; }
diff --git a/clang/lib/CIR/CodeGen/CIRGenCleanup.cpp b/clang/lib/CIR/CodeGen/CIRGenCleanup.cpp
index be21ce9c4a18c..b5a80eb7a47c7 100644
--- a/clang/lib/CIR/CodeGen/CIRGenCleanup.cpp
+++ b/clang/lib/CIR/CodeGen/CIRGenCleanup.cpp
@@ -16,6 +16,7 @@
 //
 //===----------------------------------------------------------------------===//
 
+#include "CIRGenCleanup.h"
 #include "CIRGenFunction.h"
 
 #include "clang/CIR/MissingFeatures.h"
@@ -33,6 +34,54 @@ using namespace clang::CIRGen;
 
 void EHScopeStack::Cleanup::anchor() {}
 
+/// Push an entry of the given size onto this protected-scope stack.
+char *EHScopeStack::allocate(size_t size) {
+  size = llvm::alignTo(size, ScopeStackAlignment);
+  if (!startOfBuffer) {
+    unsigned capacity = 1024;
+    while (capacity < size)
+      capacity *= 2;
+    startOfBuffer = new char[capacity];
+    startOfData = endOfBuffer = startOfBuffer + capacity;
+  } else if (static_cast<size_t>(startOfData - startOfBuffer) < size) {
+    unsigned currentCapacity = endOfBuffer - startOfBuffer;
+    unsigned usedCapacity = currentCapacity - (startOfData - startOfBuffer);
+
+    unsigned newCapacity = currentCapacity;
+    do {
+      newCapacity *= 2;
+    } while (newCapacity < usedCapacity + size);
+
+    char *newStartOfBuffer = new char[newCapacity];
+    char *newEndOfBuffer = newStartOfBuffer + newCapacity;
+    char *newStartOfData = newEndOfBuffer - usedCapacity;
+    memcpy(newStartOfData, startOfData, usedCapacity);
+    delete[] startOfBuffer;
+    startOfBuffer = newStartOfBuffer;
+    endOfBuffer = newEndOfBuffer;
+    startOfData = newStartOfData;
+  }
+
+  assert(startOfBuffer + size <= startOfData);
+  startOfData -= size;
+  return startOfData;
+}
+
+void EHScopeStack::deallocate(size_t size) {
+  startOfData += llvm::alignTo(size, ScopeStackAlignment);
+}
+
+void *EHScopeStack::pushCleanup(CleanupKind kind, size_t size) {
+  char *buffer = allocate(size);
+
+  // When the full implementation is upstreamed, this will allocate
+  // extra memory for and construct a wrapper object that is used to
+  // manage the cleanup generation.
+  assert(!cir::MissingFeatures::ehCleanupScope());
+
+  return buffer;
+}
+
 static mlir::Block *getCurCleanupBlock(CIRGenFunction &cgf) {
   mlir::OpBuilder::InsertionGuard guard(cgf.getBuilder());
   mlir::Block *cleanup =
@@ -44,26 +93,34 @@ static mlir::Block *getCurCleanupBlock(CIRGenFunction &cgf) {
 /// current insertion point is threaded through the cleanup, as are
 /// any branch fixups on the cleanup.
 void CIRGenFunction::popCleanupBlock() {
-  assert(!ehStack.cleanupStack.empty() && "cleanup stack is empty!");
+  assert(!ehStack.empty() && "cleanup stack is empty!");
+
+  // The memory for the cleanup continues to be owned by the EHScopeStack
+  // allocator, so we just destroy the object rather than attempting to
+  // free it.
+  EHScopeStack::Cleanup &cleanup = *ehStack.begin();
+
+  // The eventual implementation here will use the EHCleanupScope helper class.
+  assert(!cir::MissingFeatures::ehCleanupScope());
+
   mlir::OpBuilder::InsertionGuard guard(builder);
-  std::unique_ptr<EHScopeStack::Cleanup> cleanup =
-      ehStack.cleanupStack.pop_back_val();
 
   assert(!cir::MissingFeatures::ehCleanupFlags());
   mlir::Block *cleanupEntry = getCurCleanupBlock(*this);
   builder.setInsertionPointToEnd(cleanupEntry);
-  cleanup->emit(*this);
+  cleanup.emit(*this);
+
+  ehStack.deallocate(cleanup.getSize());
 }
 
 /// Pops cleanup blocks until the given savepoint is reached.
-void CIRGenFunction::popCleanupBlocks(size_t oldCleanupStackDepth) {
+void CIRGenFunction::popCleanupBlocks(
+    EHScopeStack::stable_iterator oldCleanupStackDepth) {
   assert(!cir::MissingFeatures::ehstackBranches());
 
-  assert(ehStack.getStackDepth() >= oldCleanupStackDepth);
-
   // Pop cleanup blocks until we reach the base stack depth for the
   // current scope.
-  while (ehStack.getStackDepth() > oldCleanupStackDepth) {
+  while (ehStack.stable_begin() != oldCleanupStackDepth) {
     popCleanupBlock();
   }
 }
diff --git a/clang/lib/CIR/CodeGen/CIRGenCleanup.h b/clang/lib/CIR/CodeGen/CIRGenCleanup.h
new file mode 100644
index 0000000000000..10918b295b4e8
--- /dev/null
+++ b/clang/lib/CIR/CodeGen/CIRGenCleanup.h
@@ -0,0 +1,43 @@
+//===----------------------------------------------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+//
+// These classes support the generation of CIR for cleanups, initially based
+// on LLVM IR cleanup handling, but ought to change as CIR evolves.
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef CLANG_LIB_CIR_CODEGEN_CGCLEANUP_H
+#define CLANG_LIB_CIR_CODEGEN_CGCLEANUP_H
+
+#include "EHScopeStack.h"
+
+namespace clang::CIRGen {
+
+/// A non-stable pointer into the scope stack.
+class EHScopeStack::iterator {
+  char *ptr;
+
+  friend class EHScopeStack;
+  explicit iterator(char *ptr) : ptr(ptr) {}
+
+public:
+  iterator() : ptr(nullptr) {}
+
+  EHScopeStack::Cleanup *get() const {
+    return reinterpret_cast<EHScopeStack::Cleanup *>(ptr);
+  }
+
+  EHScopeStack::Cleanup &operator*() const { return *get(); }
+};
+
+inline EHScopeStack::iterator EHScopeStack::begin() const {
+  return iterator(startOfData);
+}
+
+} // namespace clang::CIRGen
+#endif // CLANG_LIB_CIR_CODEGEN_CGCLEANUP_H
diff --git a/clang/lib/CIR/CodeGen/CIRGenDecl.cpp b/clang/lib/CIR/CodeGen/CIRGenDecl.cpp
index 9cdbebead5419..51bafa42730ae 100644
--- a/clang/lib/CIR/CodeGen/CIRGenDecl.cpp
+++ b/clang/lib/CIR/CodeGen/CIRGenDecl.cpp
@@ -666,6 +666,12 @@ struct DestroyObject final : EHScopeStack::Cleanup {
   void emit(CIRGenFunction &cgf) override {
     cgf.emitDestroy(addr, type, destroyer);
   }
+
+  // This is a placeholder until EHCleanupScope is implemented.
+  size_t getSize() const override {
+    assert(!cir::MissingFeatures::ehCleanupScope());
+    return sizeof(DestroyObject);
+  }
 };
 } // namespace
 
diff --git a/clang/lib/CIR/CodeGen/CIRGenFunction.cpp b/clang/lib/CIR/CodeGen/CIRGenFunction.cpp
index 3ed1e3093245b..7cbd6db96b410 100644
--- a/clang/lib/CIR/CodeGen/CIRGenFunction.cpp
+++ b/clang/lib/CIR/CodeGen/CIRGenFunction.cpp
@@ -28,8 +28,6 @@ CIRGenFunction::CIRGenFunction(CIRGenModule &cgm, CIRGenBuilderTy &builder,
                                bool suppressNewContext)
     : CIRGenTypeCache(cgm), cgm{cgm}, builder(builder) {
   ehStack.setCGF(this);
-  currentCleanupStackDepth = 0;
-  assert(ehStack.getStackDepth() == 0);
 }
 
 CIRGenFunction::~CIRGenFunction() {}
@@ -406,6 +404,8 @@ void CIRGenFunction::startFunction(GlobalDecl gd, QualType returnType,
   const auto *fd = dyn_cast_or_null<FunctionDecl>(d);
   curFuncDecl = d->getNonClosureContext();
 
+  prologueCleanupDepth = ehStack.stable_begin();
+
   mlir::Block *entryBB = &fn.getBlocks().front();
   builder.setInsertionPointToStart(entryBB);
 
@@ -472,11 +472,11 @@ void CIRGenFunction::finishFunction(SourceLocation endLoc) {
   // important to do this before we enter the return block or return
   // edges will be *really* confused.
   // TODO(cir): Use prologueCleanupDepth here.
-  bool hasCleanups = ehStack.getStackDepth() != currentCleanupStackDepth;
+  bool hasCleanups = ehStack.stable_begin() != prologueCleanupDepth;
   if (hasCleanups) {
     assert(!cir::MissingFeatures::generateDebugInfo());
     // FIXME(cir): should we clearInsertionPoint? breaks many testcases
-    popCleanupBlocks(currentCleanupStackDepth);
+    popCleanupBlocks(prologueCleanupDepth);
   }
 }
 
diff --git a/clang/lib/CIR/CodeGen/CIRGenFunction.h b/clang/lib/CIR/CodeGen/CIRGenFunction.h
index 68d54bb966cdb..9408d43d781c1 100644
--- a/clang/lib/CIR/CodeGen/CIRGenFunction.h
+++ b/clang/lib/CIR/CodeGen/CIRGenFunction.h
@@ -601,9 +601,13 @@ class CIRGenFunction : public CIRGenTypeCache {
                      FunctionArgList args, clang::SourceLocation loc,
                      clang::SourceLocation startLoc);
 
+  /// The cleanup depth enclosing all the cleanups associated with the
+  /// parameters.
+  EHScopeStack::stable_iterator prologueCleanupDepth;
+
   /// Takes the old cleanup stack size and emits the cleanup blocks
   /// that have been added.
-  void popCleanupBlocks(size_t oldCleanupStackDepth);
+  void popCleanupBlocks(EHScopeStack::stable_iterator oldCleanupStackDepth);
   void popCleanupBlock();
 
   /// Push a cleanup to be run at the end of the current full-expression.  Safe
@@ -622,7 +626,7 @@ class CIRGenFunction : public CIRGenTypeCache {
   /// Enters a new scope for capturing cleanups, all of which
   /// will be executed once the scope is exited.
   class RunCleanupsScope {
-    size_t cleanupStackDepth, oldCleanupStackDepth;
+    EHScopeStack::stable_iterator cleanupStackDepth, oldCleanupStackDepth;
 
   protected:
     bool performCleanup;
@@ -638,7 +642,7 @@ class CIRGenFunction : public CIRGenTypeCache {
     /// Enter a new cleanup scope.
     explicit RunCleanupsScope(CIRGenFunction &cgf)
         : performCleanup(true), cgf(cgf) {
-      cleanupStackDepth = cgf.ehStack.getStackDepth();
+      cleanupStackDepth = cgf.ehStack.stable_begin();
       oldCleanupStackDepth = cgf.currentCleanupStackDepth;
       cgf.currentCleanupStackDepth = cleanupStackDepth;
     }
@@ -663,7 +667,7 @@ class CIRGenFunction : public CIRGenTypeCache {
   };
 
   // Cleanup stack depth of the RunCleanupsScope that was pushed most recently.
-  size_t currentCleanupStackDepth;
+  EHScopeStack::stable_iterator currentCleanupStackDepth = ehStack.stable_end();
 
 public:
   /// Represents a scope, including function bodies, compound statements, and
diff --git a/clang/lib/CIR/CodeGen/CIRGenStmt.cpp b/clang/lib/CIR/CodeGen/CIRGenStmt.cpp
index 50642e7ca48d7..332babdf43772 100644
--- a/clang/lib/CIR/CodeGen/CIRGenStmt.cpp
+++ b/clang/lib/CIR/CodeGen/CIRGenStmt.cpp
@@ -412,7 +412,7 @@ mlir::LogicalResult CIRGenFunction::emitReturnStmt(const ReturnStmt &s) {
   auto *retBlock = curLexScope->getOrCreateRetBlock(*this, loc);
   // This should emit a branch through the cleanup block if one exists.
   builder.create<cir::BrOp>(loc, retBlock);
-  if (ehStack.getStackDepth() != currentCleanupStackDepth)
+  if (ehStack.stable_begin() != currentCleanupStackDepth)
     cgm.errorNYI(s.getSourceRange(), "return with cleanup stack");
   builder.createBlock(builder.getBlock()->getParent());
 
diff --git a/clang/lib/CIR/CodeGen/EHScopeStack.h b/clang/lib/CIR/CodeGen/EHScopeStack.h
index 22750aca3c4fc..9a1b061fff9fb 100644
--- a/clang/lib/CIR/CodeGen/EHScopeStack.h
+++ b/clang/lib/CIR/CodeGen/EHScopeStack.h
@@ -42,7 +42,47 @@ enum CleanupKind : unsigned {
 /// A stack of scopes which respond to exceptions, including cleanups
 /// and catch blocks.
 class EHScopeStack {
+  friend class CIRGenFunction;
+
 public:
+  // TODO(ogcg): Switch to alignof(uint64_t) instead of 8
+  enum { ScopeStackAlignment = 8 };
+
+  /// A saved depth on the scope stack.  This is necessary because
+  /// pushing scopes onto the stack invalidates iterators.
+  class stable_iterator {
+    friend class EHScopeStack;
+
+    /// Offset from startOfData to endOfBuffer.
+    ptrdiff_t size;
+
+    stable_iterator(ptrdiff_t size) : size(size) {}
+
+  public:
+    static stable_iterator invalid() { return stable_iterator(-1); }
+    stable_iterator() : size(-1) {}
+
+    bool isValid() const { return size >= 0; }
+
+    /// Returns true if this scope encloses I.
+    /// Returns false if I is invalid.
+    /// This scope must be valid.
+    bool encloses(stable_iterator other) const { return size <= other.size; }
+
+    /// Returns true if this scope strictly encloses I: that is,
+    /// if it encloses I and is not I.
+    /// Returns false is I is invalid.
+    /// This scope must be valid.
+    bool strictlyEncloses(stable_iterator I) const { return size < I.size; }
+
+    friend bool operator==(stable_iterator A, stable_iterator B) {
+      return A.size == B.size;
+    }
+    friend bool operator!=(stable_iterator A, stable_iterator B) {
+      return A.size != B.size;
+    }
+  };
+
   /// Information for lazily generating a cleanup.  Subclasses must be
   /// POD-like: cleanups will not be destructed, and they will be
   /// allocated on the cleanup stack and freely copied and moved
@@ -68,30 +108,75 @@ class EHScopeStack {
     ///
     // \param flags cleanup kind.
     virtual void emit(CIRGenFunction &cgf) = 0;
-  };
 
-  // Classic codegen has a finely tuned custom allocator and a complex stack
-  // management scheme. We'll probably eventually want to find a way to share
-  // that implementation. For now, we will use a very simplified implementation
-  // to get cleanups working.
-  llvm::SmallVector<std::unique_ptr<Cleanup>, 8> cleanupStack;
+    // This is a placeholder until EHScope is implemented.
+    virtual size_t getSize() const = 0;
+  };
 
 private:
+  // The implementation for this class is in CIRGenCleanup.h and
+  // CIRGenCleanup.cpp; the definition is here because it's used as a
+  // member of CIRGenFunction.
+
+  /// The start of the scope-stack buffer, i.e. the allocated pointer
+  /// for the buffer.  All of these pointers are either simultaneously
+  /// null or simultaneously valid.
+  char *startOfBuffer = nullptr;
+
+  /// The end of the buffer.
+  char *endOfBuffer = nullptr;
+
+  /// The first valid entry in the buffer.
+  char *startOfData = nullptr;
+
   /// The CGF this Stack belong to
   CIRGenFunction *cgf = nullptr;
 
+  // This class uses a custom allocator for maximum efficiency because cleanups
+  // are allocated and freed very frequently. It's basically a bump pointer
+  // allocator, but we can't use LLVM's BumpPtrAllocator because we use offsets
+  // into the buffer as stable iterators.
+  char *allocate(size_t size);
+  void deallocate(size_t size);
+
+  void *pushCleanup(CleanupKind kind, size_t dataSize);
+
 public:
   EHScopeStack() = default;
-  ~EHScopeStack() = default;
+  ~EHScopeStack() { delete[] startOfBuffer; }
 
   /// Push a lazily-created cleanup on the stack.
   template <class T, class... As> void pushCleanup(CleanupKind kind, As... a) {
-    cleanupStack.push_back(std::make_unique<T>(a...));
+    static_assert(alignof(T) <= ScopeStackAlignment,
+                  "Cleanup's alignment is too large.");
+    void *buffer = pushCleanup(kind, sizeof(T));
+    [[maybe_unused]] Cleanup *obj = new (buffer) T(a...);
   }
 
   void setCGF(CIRGenFunction *inCGF) { cgf = inCGF; }
 
-  size_t getStackDepth() const { return cleanupStack.size(); }
+  /// Pops a cleanup scope off the stack.  This is private to CIRGenCleanup.cpp.
+  void popCleanup();
+
+  /// Determines whether the exception-scopes stack is empty.
+  bool empty() const { return startOfData == endOfBuffer; }
+
+  /// An unstable reference to a scope-stack depth.  Invalidated by
+  /// pushes but not pops.
+  class iterator;
+
+  /// Returns an iterator pointing to the innermost EH scope.
+  iterator begin() const;
+
+  /// Create a stable reference to the top of the EH stack.  The
+  /// returned reference is valid until that scope is popped off the
+  /// stack.
+  stable_iterator stable_begin() const {
+    return stable_iterator(endOfBuffer - startOfData);
+  }
+
+  /// Create a stable reference to the bottom of the EH stack.
+  static stable_iterator stable_end() { return stable_iterator(0); }
 };
 
 } // namespace clang::CIRGen

Copy link
Contributor

@xlauko xlauko left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm, with minor nits

Comment on lines 14 to 15
#ifndef CLANG_LIB_CIR_CODEGEN_CGCLEANUP_H
#define CLANG_LIB_CIR_CODEGEN_CGCLEANUP_H
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
#ifndef CLANG_LIB_CIR_CODEGEN_CGCLEANUP_H
#define CLANG_LIB_CIR_CODEGEN_CGCLEANUP_H
#ifndef CLANG_LIB_CIR_CODEGEN_CIRGENCLEANUP_H
#define CLANG_LIB_CIR_CODEGEN_CIRGENCLEANUP_H

}

} // namespace clang::CIRGen
#endif // CLANG_LIB_CIR_CODEGEN_CGCLEANUP_H
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
#endif // CLANG_LIB_CIR_CODEGEN_CGCLEANUP_H
#endif // CLANG_LIB_CIR_CODEGEN_CIRGENCLEANUP_H


/// A non-stable pointer into the scope stack.
class EHScopeStack::iterator {
char *ptr;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
char *ptr;
char *ptr = nullptr;

explicit iterator(char *ptr) : ptr(ptr) {}

public:
iterator() : ptr(nullptr) {}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
iterator() : ptr(nullptr) {}
iterator() = default;

Comment on lines 50 to 53
unsigned newCapacity = currentCapacity;
do {
newCapacity *= 2;
} while (newCapacity < usedCapacity + size);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
unsigned newCapacity = currentCapacity;
do {
newCapacity *= 2;
} while (newCapacity < usedCapacity + size);
unsigned requiredCapacity = usedCapacity + size;
unsigned newCapacity = llvm::PowerOf2Ceil(std::max(requiredCapacity, currentCapacity));

Comment on lines 41 to 43
unsigned capacity = 1024;
while (capacity < size)
capacity *= 2;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
unsigned capacity = 1024;
while (capacity < size)
capacity *= 2;
unsigned capacity = llvm::PowerOf2Ceil(std::max(size, 1024u));

/// Offset from startOfData to endOfBuffer.
ptrdiff_t size;

stable_iterator(ptrdiff_t size) : size(size) {}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
stable_iterator(ptrdiff_t size) : size(size) {}
explicit stable_iterator(ptrdiff_t size) : size(size) {}

friend class EHScopeStack;

/// Offset from startOfData to endOfBuffer.
ptrdiff_t size;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
ptrdiff_t size;
ptrdiff_t size = -1;


public:
static stable_iterator invalid() { return stable_iterator(-1); }
stable_iterator() : size(-1) {}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
stable_iterator() : size(-1) {}
stable_iterator() = default;

char *newEndOfBuffer = newStartOfBuffer + newCapacity;
char *newStartOfData = newEndOfBuffer - usedCapacity;
memcpy(newStartOfData, startOfData, usedCapacity);
delete[] startOfBuffer;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we use unique_ptr at least within this function?

Copy link
Member

@bcardosolopes bcardosolopes left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, nothing to add on top of existing comments

When the cleanup handling code was initially upstreamed, a SmallVector
was used to simplify the handling of the stack of cleanup objects. However,
that mechanism won't scale well enough for the rate at which cleanup
handlers are going to be pushed and popped while compiling a large program.
This change introduces the custom memory allocator which is used in classic
codegen and the CIR incubator.

Thiis does not otherwise change the cleanup handling implementation and
many parts of the infrastructure are still missing.

This is not intended to have any observable effect on the generated CIR,
but it does change the internal implementation significantly, so it's not
exactly an NFC change. The functionality is covered by existing tests.
@andykaylor andykaylor force-pushed the cir-cleanup-allocator branch from cf0409d to ce3e429 Compare August 7, 2025 19:18
@andykaylor andykaylor merged commit ca52d9b into llvm:main Aug 7, 2025
9 checks passed
@andykaylor andykaylor deleted the cir-cleanup-allocator branch August 7, 2025 19:42
@llvm-ci
Copy link
Collaborator

llvm-ci commented Aug 7, 2025

LLVM Buildbot has detected a new failure on builder clang-hip-vega20 running on hip-vega20-0 while building clang at step 3 "annotate".

Full details are available at: https://lab.llvm.org/buildbot/#/builders/123/builds/24897

Here is the relevant piece of the build log for the reference
Step 3 (annotate) failure: '../llvm-zorg/zorg/buildbot/builders/annotated/hip-build.sh --jobs=' (failure)
...
[57/59] Linking CXX executable External/HIP/math_h-hip-6.3.0
[58/59] Building CXX object External/HIP/CMakeFiles/TheNextWeek-hip-6.3.0.dir/workload/ray-tracing/TheNextWeek/main.cc.o
[59/59] Linking CXX executable External/HIP/TheNextWeek-hip-6.3.0
+ build_step 'Testing HIP test-suite'
+ echo '@@@BUILD_STEP Testing HIP test-suite@@@'
@@@BUILD_STEP Testing HIP test-suite@@@
+ ninja check-hip-simple
[0/1] cd /home/botworker/bbot/clang-hip-vega20/botworker/clang-hip-vega20/test-suite-build/External/HIP && /home/botworker/bbot/clang-hip-vega20/botworker/clang-hip-vega20/llvm/bin/llvm-lit -sv array-hip-6.3.0.test empty-hip-6.3.0.test with-fopenmp-hip-6.3.0.test saxpy-hip-6.3.0.test memmove-hip-6.3.0.test split-kernel-args-hip-6.3.0.test builtin-logb-scalbn-hip-6.3.0.test TheNextWeek-hip-6.3.0.test algorithm-hip-6.3.0.test cmath-hip-6.3.0.test complex-hip-6.3.0.test math_h-hip-6.3.0.test new-hip-6.3.0.test blender.test
-- Testing: 14 tests, 14 workers --
Testing:  0.. 10.. 20.. 30.. 40.. 50.. 60.. 70.. 80.. 90
FAIL: test-suite :: External/HIP/blender.test (14 of 14)
******************** TEST 'test-suite :: External/HIP/blender.test' FAILED ********************

/home/botworker/bbot/clang-hip-vega20/botworker/clang-hip-vega20/test-suite-build/tools/timeit-target --timeout 7200 --limit-core 0 --limit-cpu 7200 --limit-file-size 209715200 --limit-rss-size 838860800 --append-exitstatus --redirect-output /home/botworker/bbot/clang-hip-vega20/botworker/clang-hip-vega20/test-suite-build/External/HIP/Output/blender.test.out --redirect-input /dev/null --summary /home/botworker/bbot/clang-hip-vega20/botworker/clang-hip-vega20/test-suite-build/External/HIP/Output/blender.test.time /bin/bash test_blender.sh
/bin/bash verify_blender.sh /home/botworker/bbot/clang-hip-vega20/botworker/clang-hip-vega20/test-suite-build/External/HIP/Output/blender.test.out
Begin Blender test.
TEST_SUITE_HIP_ROOT=/opt/botworker/llvm/External/hip
Render /opt/botworker/llvm/External/hip/Blender_Scenes/290skydemo_release.blend
Blender 4.1.1 (hash e1743a0317bc built 2024-04-15 23:47:45)
Read blend: "/opt/botworker/llvm/External/hip/Blender_Scenes/290skydemo_release.blend"
Could not open as Ogawa file from provided streams.
Unable to open /opt/botworker/llvm/External/hip/Blender_Scenes/290skydemo2_flags.abc
WARN (bke.modifier): source/blender/blenkernel/intern/modifier.cc:425 BKE_modifier_set_error: Object: "GEO-flag.002", Modifier: "MeshSequenceCache", Could not create reader for file //290skydemo2_flags.abc
WARN (bke.modifier): source/blender/blenkernel/intern/modifier.cc:425 BKE_modifier_set_error: Object: "GEO-flag.003", Modifier: "MeshSequenceCache", Could not create reader for file //290skydemo2_flags.abc
WARN (bke.modifier): source/blender/blenkernel/intern/modifier.cc:425 BKE_modifier_set_error: Object: "GEO-flag", Modifier: "MeshSequenceCache", Could not create reader for file //290skydemo2_flags.abc
WARN (bke.modifier): source/blender/blenkernel/intern/modifier.cc:425 BKE_modifier_set_error: Object: "GEO-flag.004", Modifier: "MeshSequenceCache", Could not create reader for file //290skydemo2_flags.abc
WARN (bke.modifier): source/blender/blenkernel/intern/modifier.cc:425 BKE_modifier_set_error: Object: "GEO-flag.001", Modifier: "MeshSequenceCache", Could not create reader for file //290skydemo2_flags.abc
Could not open as Ogawa file from provided streams.
Unable to open /opt/botworker/llvm/External/hip/Blender_Scenes/290skydemo2_flags.abc
WARN (bke.modifier): source/blender/blenkernel/intern/modifier.cc:425 BKE_modifier_set_error: Object: "GEO-flag.002", Modifier: "MeshSequenceCache", Could not create reader for file //290skydemo2_flags.abc
WARN (bke.modifier): source/blender/blenkernel/intern/modifier.cc:425 BKE_modifier_set_error: Object: "GEO-flag.003", Modifier: "MeshSequenceCache", Could not create reader for file //290skydemo2_flags.abc
WARN (bke.modifier): source/blender/blenkernel/intern/modifier.cc:425 BKE_modifier_set_error: Object: "GEO-flag", Modifier: "MeshSequenceCache", Could not create reader for file //290skydemo2_flags.abc
WARN (bke.modifier): source/blender/blenkernel/intern/modifier.cc:425 BKE_modifier_set_error: Object: "GEO-flag.004", Modifier: "MeshSequenceCache", Could not create reader for file //290skydemo2_flags.abc
WARN (bke.modifier): source/blender/blenkernel/intern/modifier.cc:425 BKE_modifier_set_error: Object: "GEO-flag.001", Modifier: "MeshSequenceCache", Could not create reader for file //290skydemo2_flags.abc
I0807 19:49:15.298236 624595 device.cpp:39] HIPEW initialization succeeded
I0807 19:49:15.300192 624595 device.cpp:45] Found HIPCC hipcc
I0807 19:49:15.358426 624595 device.cpp:207] Device has compute preemption or is not used for display.
I0807 19:49:15.358495 624595 device.cpp:211] Added device "" with id "HIP__0000:a3:00".
I0807 19:49:15.358580 624595 device.cpp:568] Mapped host memory limit set to 536,444,985,344 bytes. (499.60G)
I0807 19:49:15.358819 624595 device_impl.cpp:63] Using AVX2 CPU kernels.
Fra:1 Mem:524.00M (Peak 524.70M) | Time:00:00.68 | Mem:0.00M, Peak:0.00M | Scene, View Layer | Synchronizing object | GEO-Eyepiece_rim
Fra:1 Mem:524.00M (Peak 524.70M) | Time:00:00.68 | Mem:0.00M, Peak:0.00M | Scene, View Layer | Synchronizing object | GEO-Rivets.006
Fra:1 Mem:524.05M (Peak 524.70M) | Time:00:00.68 | Mem:0.00M, Peak:0.00M | Scene, View Layer | Synchronizing object | GEO-Curve_Cables.004
Fra:1 Mem:524.08M (Peak 524.70M) | Time:00:00.68 | Mem:0.00M, Peak:0.00M | Scene, View Layer | Synchronizing object | GEO-Rivets.024
Fra:1 Mem:524.25M (Peak 524.70M) | Time:00:00.68 | Mem:0.00M, Peak:0.00M | Scene, View Layer | Synchronizing object | GEO-Hoses.003
Fra:1 Mem:524.64M (Peak 524.70M) | Time:00:00.68 | Mem:0.00M, Peak:0.00M | Scene, View Layer | Synchronizing object | GEO-Curve_Connectors
Fra:1 Mem:525.38M (Peak 525.38M) | Time:00:00.68 | Mem:0.00M, Peak:0.00M | Scene, View Layer | Synchronizing object | GEO-Curve_Wires
Fra:1 Mem:525.69M (Peak 525.69M) | Time:00:00.68 | Mem:0.00M, Peak:0.00M | Scene, View Layer | Synchronizing object | GEO-Curve_Connectors.001
Fra:1 Mem:525.73M (Peak 525.73M) | Time:00:00.68 | Mem:0.00M, Peak:0.00M | Scene, View Layer | Synchronizing object | GEO-Curve_Connectors.002
Step 12 (Testing HIP test-suite) failure: Testing HIP test-suite (failure)
@@@BUILD_STEP Testing HIP test-suite@@@
+ ninja check-hip-simple
[0/1] cd /home/botworker/bbot/clang-hip-vega20/botworker/clang-hip-vega20/test-suite-build/External/HIP && /home/botworker/bbot/clang-hip-vega20/botworker/clang-hip-vega20/llvm/bin/llvm-lit -sv array-hip-6.3.0.test empty-hip-6.3.0.test with-fopenmp-hip-6.3.0.test saxpy-hip-6.3.0.test memmove-hip-6.3.0.test split-kernel-args-hip-6.3.0.test builtin-logb-scalbn-hip-6.3.0.test TheNextWeek-hip-6.3.0.test algorithm-hip-6.3.0.test cmath-hip-6.3.0.test complex-hip-6.3.0.test math_h-hip-6.3.0.test new-hip-6.3.0.test blender.test
-- Testing: 14 tests, 14 workers --
Testing:  0.. 10.. 20.. 30.. 40.. 50.. 60.. 70.. 80.. 90
FAIL: test-suite :: External/HIP/blender.test (14 of 14)
******************** TEST 'test-suite :: External/HIP/blender.test' FAILED ********************

/home/botworker/bbot/clang-hip-vega20/botworker/clang-hip-vega20/test-suite-build/tools/timeit-target --timeout 7200 --limit-core 0 --limit-cpu 7200 --limit-file-size 209715200 --limit-rss-size 838860800 --append-exitstatus --redirect-output /home/botworker/bbot/clang-hip-vega20/botworker/clang-hip-vega20/test-suite-build/External/HIP/Output/blender.test.out --redirect-input /dev/null --summary /home/botworker/bbot/clang-hip-vega20/botworker/clang-hip-vega20/test-suite-build/External/HIP/Output/blender.test.time /bin/bash test_blender.sh
/bin/bash verify_blender.sh /home/botworker/bbot/clang-hip-vega20/botworker/clang-hip-vega20/test-suite-build/External/HIP/Output/blender.test.out
Begin Blender test.
TEST_SUITE_HIP_ROOT=/opt/botworker/llvm/External/hip
Render /opt/botworker/llvm/External/hip/Blender_Scenes/290skydemo_release.blend
Blender 4.1.1 (hash e1743a0317bc built 2024-04-15 23:47:45)
Read blend: "/opt/botworker/llvm/External/hip/Blender_Scenes/290skydemo_release.blend"
Could not open as Ogawa file from provided streams.
Unable to open /opt/botworker/llvm/External/hip/Blender_Scenes/290skydemo2_flags.abc
WARN (bke.modifier): source/blender/blenkernel/intern/modifier.cc:425 BKE_modifier_set_error: Object: "GEO-flag.002", Modifier: "MeshSequenceCache", Could not create reader for file //290skydemo2_flags.abc
WARN (bke.modifier): source/blender/blenkernel/intern/modifier.cc:425 BKE_modifier_set_error: Object: "GEO-flag.003", Modifier: "MeshSequenceCache", Could not create reader for file //290skydemo2_flags.abc
WARN (bke.modifier): source/blender/blenkernel/intern/modifier.cc:425 BKE_modifier_set_error: Object: "GEO-flag", Modifier: "MeshSequenceCache", Could not create reader for file //290skydemo2_flags.abc
WARN (bke.modifier): source/blender/blenkernel/intern/modifier.cc:425 BKE_modifier_set_error: Object: "GEO-flag.004", Modifier: "MeshSequenceCache", Could not create reader for file //290skydemo2_flags.abc
WARN (bke.modifier): source/blender/blenkernel/intern/modifier.cc:425 BKE_modifier_set_error: Object: "GEO-flag.001", Modifier: "MeshSequenceCache", Could not create reader for file //290skydemo2_flags.abc
Could not open as Ogawa file from provided streams.
Unable to open /opt/botworker/llvm/External/hip/Blender_Scenes/290skydemo2_flags.abc
WARN (bke.modifier): source/blender/blenkernel/intern/modifier.cc:425 BKE_modifier_set_error: Object: "GEO-flag.002", Modifier: "MeshSequenceCache", Could not create reader for file //290skydemo2_flags.abc
WARN (bke.modifier): source/blender/blenkernel/intern/modifier.cc:425 BKE_modifier_set_error: Object: "GEO-flag.003", Modifier: "MeshSequenceCache", Could not create reader for file //290skydemo2_flags.abc
WARN (bke.modifier): source/blender/blenkernel/intern/modifier.cc:425 BKE_modifier_set_error: Object: "GEO-flag", Modifier: "MeshSequenceCache", Could not create reader for file //290skydemo2_flags.abc
WARN (bke.modifier): source/blender/blenkernel/intern/modifier.cc:425 BKE_modifier_set_error: Object: "GEO-flag.004", Modifier: "MeshSequenceCache", Could not create reader for file //290skydemo2_flags.abc
WARN (bke.modifier): source/blender/blenkernel/intern/modifier.cc:425 BKE_modifier_set_error: Object: "GEO-flag.001", Modifier: "MeshSequenceCache", Could not create reader for file //290skydemo2_flags.abc
I0807 19:49:15.298236 624595 device.cpp:39] HIPEW initialization succeeded
I0807 19:49:15.300192 624595 device.cpp:45] Found HIPCC hipcc
I0807 19:49:15.358426 624595 device.cpp:207] Device has compute preemption or is not used for display.
I0807 19:49:15.358495 624595 device.cpp:211] Added device "" with id "HIP__0000:a3:00".
I0807 19:49:15.358580 624595 device.cpp:568] Mapped host memory limit set to 536,444,985,344 bytes. (499.60G)
I0807 19:49:15.358819 624595 device_impl.cpp:63] Using AVX2 CPU kernels.
Fra:1 Mem:524.00M (Peak 524.70M) | Time:00:00.68 | Mem:0.00M, Peak:0.00M | Scene, View Layer | Synchronizing object | GEO-Eyepiece_rim
Fra:1 Mem:524.00M (Peak 524.70M) | Time:00:00.68 | Mem:0.00M, Peak:0.00M | Scene, View Layer | Synchronizing object | GEO-Rivets.006
Fra:1 Mem:524.05M (Peak 524.70M) | Time:00:00.68 | Mem:0.00M, Peak:0.00M | Scene, View Layer | Synchronizing object | GEO-Curve_Cables.004
Fra:1 Mem:524.08M (Peak 524.70M) | Time:00:00.68 | Mem:0.00M, Peak:0.00M | Scene, View Layer | Synchronizing object | GEO-Rivets.024
Fra:1 Mem:524.25M (Peak 524.70M) | Time:00:00.68 | Mem:0.00M, Peak:0.00M | Scene, View Layer | Synchronizing object | GEO-Hoses.003
Fra:1 Mem:524.64M (Peak 524.70M) | Time:00:00.68 | Mem:0.00M, Peak:0.00M | Scene, View Layer | Synchronizing object | GEO-Curve_Connectors
Fra:1 Mem:525.38M (Peak 525.38M) | Time:00:00.68 | Mem:0.00M, Peak:0.00M | Scene, View Layer | Synchronizing object | GEO-Curve_Wires
Fra:1 Mem:525.69M (Peak 525.69M) | Time:00:00.68 | Mem:0.00M, Peak:0.00M | Scene, View Layer | Synchronizing object | GEO-Curve_Connectors.001
Fra:1 Mem:525.73M (Peak 525.73M) | Time:00:00.68 | Mem:0.00M, Peak:0.00M | Scene, View Layer | Synchronizing object | GEO-Curve_Connectors.002
Fra:1 Mem:526.70M (Peak 526.83M) | Time:00:00.68 | Mem:0.00M, Peak:0.00M | Scene, View Layer | Synchronizing object | GEO-Curve_wires
Fra:1 Mem:527.10M (Peak 527.10M) | Time:00:00.68 | Mem:0.00M, Peak:0.00M | Scene, View Layer | Synchronizing object | GEO-Curve_Connectors.004
Fra:1 Mem:528.11M (Peak 528.11M) | Time:00:00.68 | Mem:0.00M, Peak:0.00M | Scene, View Layer | Synchronizing object | GEO-Curve_Connectors.006
Fra:1 Mem:529.10M (Peak 529.10M) | Time:00:00.68 | Mem:0.00M, Peak:0.00M | Scene, View Layer | Synchronizing object | GEO-Curve_Connectors.007
Fra:1 Mem:529.64M (Peak 529.64M) | Time:00:00.68 | Mem:0.00M, Peak:0.00M | Scene, View Layer | Synchronizing object | GEO-Curve_Connectors.008
Fra:1 Mem:530.37M (Peak 530.37M) | Time:00:00.68 | Mem:0.00M, Peak:0.00M | Scene, View Layer | Synchronizing object | GEO-Curve_Connectors.009

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clang Clang issues not falling into any other category ClangIR Anything related to the ClangIR project
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants