https://github.com/tbaederr created https://github.com/llvm/llvm-project/pull/125349
…5325) Move the BottomFrame to InterpState instead. >From 1f491bfbf6dd0bf81c7d531920276c5f6ac2c04d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Timm=20B=C3=A4der?= <tbae...@redhat.com> Date: Sat, 1 Feb 2025 06:55:55 +0100 Subject: [PATCH] Reapply "[clang][bytecode] Stack-allocate bottom function frame" (#125325) Move the BottomFrame to InterpState instead. --- clang/lib/AST/ByteCode/Context.cpp | 5 +---- clang/lib/AST/ByteCode/EvalEmitter.cpp | 6 +----- clang/lib/AST/ByteCode/Interp.h | 8 ++++---- clang/lib/AST/ByteCode/InterpFrame.cpp | 20 ++++++++++++++------ clang/lib/AST/ByteCode/InterpFrame.h | 12 ++++++++++++ clang/lib/AST/ByteCode/InterpState.cpp | 12 ++++++++++-- clang/lib/AST/ByteCode/InterpState.h | 4 ++++ 7 files changed, 46 insertions(+), 21 deletions(-) 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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits