[clang] [clang][Interp] Cleaning up `FIXME`s added during `ArrayInitLoopExpr` implementation. (PR #70053)
via cfe-commits
cfe-commits at lists.llvm.org
Thu Jan 25 06:53:31 PST 2024
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+isuckatcs at users.noreply.github.com>
Date: Sat, 30 Sep 2023 17:05:02 +0200
Subject: [PATCH 1/3] 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 1b33c69b93aa4b..8d18698df60c2c 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 83986d3dd579ed..774d0b25f4ad56 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+isuckatcs at users.noreply.github.com>
Date: Wed, 24 Jan 2024 18:19:16 +0100
Subject: [PATCH 2/3] 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 774d0b25f4ad56..8a5f644c227ffc 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+isuckatcs at users.noreply.github.com>
Date: Thu, 25 Jan 2024 15:48:26 +0100
Subject: [PATCH 3/3] 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 8a5f644c227ffc..8fb8c38877bcde 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);
};
}
More information about the cfe-commits
mailing list