https://github.com/isuckatcs updated https://github.com/llvm/llvm-project/pull/70053
>From 7cca7a3a6d969318fb8531751f75bb41715c7475 Mon Sep 17 00:00:00 2001 From: isuckatcs <65320245+isucka...@users.noreply.github.com> Date: Sat, 30 Sep 2023 17:05:02 +0200 Subject: [PATCH 1/4] cleanup --- clang/lib/AST/Interp/ByteCodeExprGen.cpp | 20 ++++++++------------ clang/lib/AST/Interp/ByteCodeExprGen.h | 20 ++++++++++++++++++++ 2 files changed, 28 insertions(+), 12 deletions(-) diff --git a/clang/lib/AST/Interp/ByteCodeExprGen.cpp b/clang/lib/AST/Interp/ByteCodeExprGen.cpp index 1b33c69b93aa4b9..8d18698df60c2c1 100644 --- a/clang/lib/AST/Interp/ByteCodeExprGen.cpp +++ b/clang/lib/AST/Interp/ByteCodeExprGen.cpp @@ -816,22 +816,18 @@ bool ByteCodeExprGen<Emitter>::VisitArrayInitLoopExpr( const ArrayInitLoopExpr *E) { assert(Initializing); assert(!DiscardResult); + + // We visit the common opaque expression here once so we have its value + // cached. + if (!this->discard(E->getCommonExpr())) + return false; + // TODO: This compiles to quite a lot of bytecode if the array is larger. // Investigate compiling this to a loop. - const Expr *SubExpr = E->getSubExpr(); - const Expr *CommonExpr = E->getCommonExpr(); size_t Size = E->getArraySize().getZExtValue(); std::optional<PrimType> ElemT = classify(SubExpr->getType()); - // If the common expression is an opaque expression, we visit it - // here once so we have its value cached. - // FIXME: This might be necessary (or useful) for all expressions. - if (isa<OpaqueValueExpr>(CommonExpr)) { - if (!this->discard(CommonExpr)) - return false; - } - // So, every iteration, we execute an assignment here // where the LHS is on the stack (the target array) // and the RHS is our SubExpr. @@ -882,13 +878,13 @@ bool ByteCodeExprGen<Emitter>::VisitOpaqueValueExpr(const OpaqueValueExpr *E) { return false; // Here the local variable is created but the value is removed from the stack, - // so we put it back, because the caller might need it. + // so we put it back if the caller needs it. if (!DiscardResult) { if (!this->emitGetLocal(SubExprT, *LocalIndex, E)) return false; } - // FIXME: Ideally the cached value should be cleaned up later. + // This is cleaned up when the local variable is destroyed. OpaqueExprs.insert({E, *LocalIndex}); return true; diff --git a/clang/lib/AST/Interp/ByteCodeExprGen.h b/clang/lib/AST/Interp/ByteCodeExprGen.h index 83986d3dd579ed6..774d0b25f4ad56d 100644 --- a/clang/lib/AST/Interp/ByteCodeExprGen.h +++ b/clang/lib/AST/Interp/ByteCodeExprGen.h @@ -360,6 +360,7 @@ template <class Emitter> class LocalScope : public VariableScope<Emitter> { if (!Idx) return; this->Ctx->emitDestroy(*Idx, SourceInfo{}); + removeStoredOpaqueValues(); } /// Overriden to support explicit destruction. @@ -368,6 +369,7 @@ template <class Emitter> class LocalScope : public VariableScope<Emitter> { return; this->emitDestructors(); this->Ctx->emitDestroy(*Idx, SourceInfo{}); + removeStoredOpaqueValues(); this->Idx = std::nullopt; } @@ -389,10 +391,28 @@ template <class Emitter> class LocalScope : public VariableScope<Emitter> { if (!Local.Desc->isPrimitive() && !Local.Desc->isPrimitiveArray()) { this->Ctx->emitGetPtrLocal(Local.Offset, SourceInfo{}); this->Ctx->emitRecordDestruction(Local.Desc); + removeIfStoredOpaqueValue(Local); } } } + void removeStoredOpaqueValues() { + if (!Idx) + return; + + for (Scope::Local &Local : this->Ctx->Descriptors[*Idx]) { + removeIfStoredOpaqueValue(Local); + } + } + + void removeIfStoredOpaqueValue(const Scope::Local &Local) { + if (auto *OVE = + llvm::dyn_cast_or_null<OpaqueValueExpr>(Local.Desc->asExpr()); + OVE && this->Ctx->OpaqueExprs.contains(OVE)) { + this->Ctx->OpaqueExprs.erase(OVE); + }; + } + /// Index of the scope in the chain. std::optional<unsigned> Idx; }; >From 39f10263f95b60441bb6df032f0e89fa57fe153e Mon Sep 17 00:00:00 2001 From: isuckatcs <65320245+isucka...@users.noreply.github.com> Date: Wed, 24 Jan 2024 18:19:16 +0100 Subject: [PATCH 2/4] addressed comments --- clang/lib/AST/Interp/ByteCodeExprGen.h | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/clang/lib/AST/Interp/ByteCodeExprGen.h b/clang/lib/AST/Interp/ByteCodeExprGen.h index 774d0b25f4ad56d..8a5f644c227ffc0 100644 --- a/clang/lib/AST/Interp/ByteCodeExprGen.h +++ b/clang/lib/AST/Interp/ByteCodeExprGen.h @@ -54,7 +54,7 @@ class ByteCodeExprGen : public ConstStmtVisitor<ByteCodeExprGen<Emitter>, bool>, public: /// Initializes the compiler and the backend emitter. template <typename... Tys> - ByteCodeExprGen(Context &Ctx, Program &P, Tys &&... Args) + ByteCodeExprGen(Context &Ctx, Program &P, Tys &&...Args) : Emitter(Ctx, P, Args...), Ctx(Ctx), P(P) {} // Expression visitors - result returned on interp stack. @@ -241,8 +241,7 @@ class ByteCodeExprGen : public ConstStmtVisitor<ByteCodeExprGen<Emitter>, bool>, llvm::function_ref<bool(PrimType)> Direct, llvm::function_ref<bool(PrimType)> Indirect); bool dereferenceParam(const Expr *LV, PrimType T, const ParmVarDecl *PD, - DerefKind AK, - llvm::function_ref<bool(PrimType)> Direct, + DerefKind AK, llvm::function_ref<bool(PrimType)> Direct, llvm::function_ref<bool(PrimType)> Indirect); bool dereferenceVar(const Expr *LV, PrimType T, const VarDecl *PD, DerefKind AK, llvm::function_ref<bool(PrimType)> Direct, @@ -400,16 +399,17 @@ template <class Emitter> class LocalScope : public VariableScope<Emitter> { if (!Idx) return; - for (Scope::Local &Local : this->Ctx->Descriptors[*Idx]) { + for (const Scope::Local &Local : this->Ctx->Descriptors[*Idx]) { removeIfStoredOpaqueValue(Local); } } void removeIfStoredOpaqueValue(const Scope::Local &Local) { if (auto *OVE = - llvm::dyn_cast_or_null<OpaqueValueExpr>(Local.Desc->asExpr()); - OVE && this->Ctx->OpaqueExprs.contains(OVE)) { - this->Ctx->OpaqueExprs.erase(OVE); + llvm::dyn_cast_if_present<OpaqueValueExpr>(Local.Desc->asExpr())) { + if (auto it = this->Ctx->OpaqueExprs.find(OVE); + it != this->Ctx->OpaqueExprs.end()) + this->Ctx->OpaqueExprs.erase(it); }; } >From e8cf91be3d669199f22914cc199e107d88cebcf4 Mon Sep 17 00:00:00 2001 From: isuckatcs <65320245+isucka...@users.noreply.github.com> Date: Thu, 25 Jan 2024 15:48:26 +0100 Subject: [PATCH 3/4] addressed comments --- clang/lib/AST/Interp/ByteCodeExprGen.h | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/clang/lib/AST/Interp/ByteCodeExprGen.h b/clang/lib/AST/Interp/ByteCodeExprGen.h index 8a5f644c227ffc0..8fb8c38877bcdea 100644 --- a/clang/lib/AST/Interp/ByteCodeExprGen.h +++ b/clang/lib/AST/Interp/ByteCodeExprGen.h @@ -405,11 +405,11 @@ template <class Emitter> class LocalScope : public VariableScope<Emitter> { } void removeIfStoredOpaqueValue(const Scope::Local &Local) { - if (auto *OVE = + if (const auto *OVE = llvm::dyn_cast_if_present<OpaqueValueExpr>(Local.Desc->asExpr())) { - if (auto it = this->Ctx->OpaqueExprs.find(OVE); - it != this->Ctx->OpaqueExprs.end()) - this->Ctx->OpaqueExprs.erase(it); + if (auto It = this->Ctx->OpaqueExprs.find(OVE); + It != this->Ctx->OpaqueExprs.end()) + this->Ctx->OpaqueExprs.erase(It); }; } >From 57f9850d99ae2cc76241bcd750919a467e0dbe74 Mon Sep 17 00:00:00 2001 From: isuckatcs <65320245+isucka...@users.noreply.github.com> Date: Thu, 25 Jan 2024 15:58:25 +0100 Subject: [PATCH 4/4] remove unrelated formatting --- clang/lib/AST/Interp/ByteCodeExprGen.h | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/clang/lib/AST/Interp/ByteCodeExprGen.h b/clang/lib/AST/Interp/ByteCodeExprGen.h index 8fb8c38877bcdea..0a2237061c6a261 100644 --- a/clang/lib/AST/Interp/ByteCodeExprGen.h +++ b/clang/lib/AST/Interp/ByteCodeExprGen.h @@ -54,7 +54,7 @@ class ByteCodeExprGen : public ConstStmtVisitor<ByteCodeExprGen<Emitter>, bool>, public: /// Initializes the compiler and the backend emitter. template <typename... Tys> - ByteCodeExprGen(Context &Ctx, Program &P, Tys &&...Args) + ByteCodeExprGen(Context &Ctx, Program &P, Tys &&... Args) : Emitter(Ctx, P, Args...), Ctx(Ctx), P(P) {} // Expression visitors - result returned on interp stack. @@ -241,7 +241,8 @@ class ByteCodeExprGen : public ConstStmtVisitor<ByteCodeExprGen<Emitter>, bool>, llvm::function_ref<bool(PrimType)> Direct, llvm::function_ref<bool(PrimType)> Indirect); bool dereferenceParam(const Expr *LV, PrimType T, const ParmVarDecl *PD, - DerefKind AK, llvm::function_ref<bool(PrimType)> Direct, + DerefKind AK, + llvm::function_ref<bool(PrimType)> Direct, llvm::function_ref<bool(PrimType)> Indirect); bool dereferenceVar(const Expr *LV, PrimType T, const VarDecl *PD, DerefKind AK, llvm::function_ref<bool(PrimType)> Direct, _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits