Reapply "[clang][bytecode] Stack-allocate bottom function frame" (#12…

[tbaederr]

…5325)

Move the BottomFrame to InterpState instead.

[llvmbot]

@llvm/pr-subscribers-clang

Author: Timm Baeder (tbaederr)

Changes

…5325)

Move the BottomFrame to InterpState instead.

---

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

7 Files Affected:

• (modified) clang/lib/AST/ByteCode/Context.cpp (+1-4)
• (modified) clang/lib/AST/ByteCode/EvalEmitter.cpp (+1-5)
• (modified) clang/lib/AST/ByteCode/Interp.h (+4-4)
• (modified) clang/lib/AST/ByteCode/InterpFrame.cpp (+14-6)
• (modified) clang/lib/AST/ByteCode/InterpFrame.h (+12)
• (modified) clang/lib/AST/ByteCode/InterpState.cpp (+10-2)
• (modified) clang/lib/AST/ByteCode/InterpState.h (+4)

diff --git a/clang/lib/AST/ByteCode/Context.cpp b/clang/lib/AST/ByteCode/Context.cpp
index a322700fc0d229..aa434d5c85921f 100644
--- a/clang/lib/AST/ByteCode/Context.cpp
+++ b/clang/lib/AST/ByteCode/Context.cpp
@@ -212,14 +212,11 @@ const llvm::fltSemantics &Context::getFloatSemantics(QualType T) const {
 bool Context::Run(State &Parent, const Function *Func) {
 
   {
-    InterpState State(Parent, *P, Stk, *this);
-    State.Current = new InterpFrame(State, Func, /*Caller=*/nullptr, CodePtr(),
-                                    Func->getArgSize());
+    InterpState State(Parent, *P, Stk, *this, Func);
     if (Interpret(State)) {
       assert(Stk.empty());
       return true;
     }
-
     // State gets destroyed here, so the Stk.clear() below doesn't accidentally
     // remove values the State's destructor might access.
   }
diff --git a/clang/lib/AST/ByteCode/EvalEmitter.cpp b/clang/lib/AST/ByteCode/EvalEmitter.cpp
index 8134bbf270363e..95149efbea9921 100644
--- a/clang/lib/AST/ByteCode/EvalEmitter.cpp
+++ b/clang/lib/AST/ByteCode/EvalEmitter.cpp
@@ -17,11 +17,7 @@ using namespace clang::interp;
 
 EvalEmitter::EvalEmitter(Context &Ctx, Program &P, State &Parent,
                          InterpStack &Stk)
-    : Ctx(Ctx), P(P), S(Parent, P, Stk, Ctx, this), EvalResult(&Ctx) {
-  // Create a dummy frame for the interpreter which does not have locals.
-  S.Current =
-      new InterpFrame(S, /*Func=*/nullptr, /*Caller=*/nullptr, CodePtr(), 0);
-}
+    : Ctx(Ctx), P(P), S(Parent, P, Stk, Ctx, this), EvalResult(&Ctx) {}
 
 EvalEmitter::~EvalEmitter() {
   for (auto &[K, V] : Locals) {
diff --git a/clang/lib/AST/ByteCode/Interp.h b/clang/lib/AST/ByteCode/Interp.h
index 063970afec9e35..2a39e3932077f5 100644
--- a/clang/lib/AST/ByteCode/Interp.h
+++ b/clang/lib/AST/ByteCode/Interp.h
@@ -325,11 +325,11 @@ bool Ret(InterpState &S, CodePtr &PC) {
 
   if (InterpFrame *Caller = S.Current->Caller) {
     PC = S.Current->getRetPC();
-    delete S.Current;
+    InterpFrame::free(S.Current);
     S.Current = Caller;
     S.Stk.push<T>(Ret);
   } else {
-    delete S.Current;
+    InterpFrame::free(S.Current);
     S.Current = nullptr;
     // The topmost frame should come from an EvalEmitter,
     // which has its own implementation of the Ret<> instruction.
@@ -345,10 +345,10 @@ inline bool RetVoid(InterpState &S, CodePtr &PC) {
 
   if (InterpFrame *Caller = S.Current->Caller) {
     PC = S.Current->getRetPC();
-    delete S.Current;
+    InterpFrame::free(S.Current);
     S.Current = Caller;
   } else {
-    delete S.Current;
+    InterpFrame::free(S.Current);
     S.Current = nullptr;
   }
   return true;
diff --git a/clang/lib/AST/ByteCode/InterpFrame.cpp b/clang/lib/AST/ByteCode/InterpFrame.cpp
index 48a3db055c6c9b..89fc7a4515d641 100644
--- a/clang/lib/AST/ByteCode/InterpFrame.cpp
+++ b/clang/lib/AST/ByteCode/InterpFrame.cpp
@@ -23,11 +23,15 @@
 using namespace clang;
 using namespace clang::interp;
 
+InterpFrame::InterpFrame(InterpState &S)
+    : Caller(nullptr), S(S), Depth(0), Func(nullptr), RetPC(CodePtr()),
+      ArgSize(0), Args(nullptr), FrameOffset(0), IsBottom(true) {}
+
 InterpFrame::InterpFrame(InterpState &S, const Function *Func,
                          InterpFrame *Caller, CodePtr RetPC, unsigned ArgSize)
     : Caller(Caller), S(S), Depth(Caller ? Caller->Depth + 1 : 0), Func(Func),
       RetPC(RetPC), ArgSize(ArgSize), Args(static_cast<char *>(S.Stk.top())),
-      FrameOffset(S.Stk.size()) {
+      FrameOffset(S.Stk.size()), IsBottom(!Caller) {
   if (!Func)
     return;
 
@@ -73,11 +77,15 @@ InterpFrame::~InterpFrame() {
   // When destroying the InterpFrame, call the Dtor for all block
   // that haven't been destroyed via a destroy() op yet.
   // This happens when the execution is interruped midway-through.
-  if (Func) {
-    for (auto &Scope : Func->scopes()) {
-      for (auto &Local : Scope.locals()) {
-        S.deallocate(localBlock(Local.Offset));
-      }
+  destroyScopes();
+}
+
+void InterpFrame::destroyScopes() {
+  if (!Func)
+    return;
+  for (auto &Scope : Func->scopes()) {
+    for (auto &Local : Scope.locals()) {
+      S.deallocate(localBlock(Local.Offset));
     }
   }
 }
diff --git a/clang/lib/AST/ByteCode/InterpFrame.h b/clang/lib/AST/ByteCode/InterpFrame.h
index 7cfc3ac68b4f3e..360e6bff12327a 100644
--- a/clang/lib/AST/ByteCode/InterpFrame.h
+++ b/clang/lib/AST/ByteCode/InterpFrame.h
@@ -28,6 +28,9 @@ class InterpFrame final : public Frame {
   /// The frame of the previous function.
   InterpFrame *Caller;
 
+  /// Bottom Frame.
+  InterpFrame(InterpState &S);
+
   /// Creates a new frame for a method call.
   InterpFrame(InterpState &S, const Function *Func, InterpFrame *Caller,
               CodePtr RetPC, unsigned ArgSize);
@@ -42,9 +45,15 @@ class InterpFrame final : public Frame {
   /// Destroys the frame, killing all live pointers to stack slots.
   ~InterpFrame();
 
+  static void free(InterpFrame *F) {
+    if (!F->isBottomFrame())
+      delete F;
+  }
+
   /// Invokes the destructors for a scope.
   void destroy(unsigned Idx);
   void initScope(unsigned Idx);
+  void destroyScopes();
 
   /// Describes the frame with arguments for diagnostic purposes.
   void describe(llvm::raw_ostream &OS) const override;
@@ -119,6 +128,8 @@ class InterpFrame final : public Frame {
 
   bool isStdFunction() const;
 
+  bool isBottomFrame() const { return IsBottom; }
+
   void dump() const { dump(llvm::errs(), 0); }
   void dump(llvm::raw_ostream &OS, unsigned Indent = 0) const;
 
@@ -167,6 +178,7 @@ class InterpFrame final : public Frame {
   const size_t FrameOffset;
   /// Mapping from arg offsets to their argument blocks.
   llvm::DenseMap<unsigned, std::unique_ptr<char[]>> Params;
+  bool IsBottom = false;
 };
 
 } // namespace interp
diff --git a/clang/lib/AST/ByteCode/InterpState.cpp b/clang/lib/AST/ByteCode/InterpState.cpp
index 287c3bd3bca3a5..70a2e9b62fc3ad 100644
--- a/clang/lib/AST/ByteCode/InterpState.cpp
+++ b/clang/lib/AST/ByteCode/InterpState.cpp
@@ -17,7 +17,14 @@ using namespace clang::interp;
 
 InterpState::InterpState(State &Parent, Program &P, InterpStack &Stk,
                          Context &Ctx, SourceMapper *M)
-    : Parent(Parent), M(M), P(P), Stk(Stk), Ctx(Ctx), Current(nullptr) {}
+    : Parent(Parent), M(M), P(P), Stk(Stk), Ctx(Ctx), BottomFrame(*this),
+      Current(&BottomFrame) {}
+
+InterpState::InterpState(State &Parent, Program &P, InterpStack &Stk,
+                         Context &Ctx, const Function *Func)
+    : Parent(Parent), M(nullptr), P(P), Stk(Stk), Ctx(Ctx),
+      BottomFrame(*this, Func, nullptr, CodePtr(), Func->getArgSize()),
+      Current(&BottomFrame) {}
 
 bool InterpState::inConstantContext() const {
   if (ConstantContextOverride)
@@ -27,11 +34,12 @@ bool InterpState::inConstantContext() const {
 }
 
 InterpState::~InterpState() {
-  while (Current) {
+  while (Current && !Current->isBottomFrame()) {
     InterpFrame *Next = Current->Caller;
     delete Current;
     Current = Next;
   }
+  BottomFrame.destroyScopes();
 
   while (DeadBlocks) {
     DeadBlock *Next = DeadBlocks->Next;
diff --git a/clang/lib/AST/ByteCode/InterpState.h b/clang/lib/AST/ByteCode/InterpState.h
index 2a1311c86a2f2a..d6adfff1a713ac 100644
--- a/clang/lib/AST/ByteCode/InterpState.h
+++ b/clang/lib/AST/ByteCode/InterpState.h
@@ -37,6 +37,8 @@ class InterpState final : public State, public SourceMapper {
 public:
   InterpState(State &Parent, Program &P, InterpStack &Stk, Context &Ctx,
               SourceMapper *M = nullptr);
+  InterpState(State &Parent, Program &P, InterpStack &Stk, Context &Ctx,
+              const Function *Func);
 
   ~InterpState();
 
@@ -134,6 +136,8 @@ class InterpState final : public State, public SourceMapper {
   InterpStack &Stk;
   /// Interpreter Context.
   Context &Ctx;
+  /// Bottom function frame.
+  InterpFrame BottomFrame;
   /// The current frame.
   InterpFrame *Current = nullptr;
   /// Source location of the evaluating expression