[clang] 3665da3 - Re-commit "[clang][Interp] Unify visiting variable declarations"
Timm Bäder via cfe-commits
cfe-commits at lists.llvm.org
Sat Jan 21 01:24:07 PST 2023
Author: Timm Bäder
Date: 2023-01-21T10:23:53+01:00
New Revision: 3665da3d0091ab765d54ce643bd82d353c040631
URL: https://github.com/llvm/llvm-project/commit/3665da3d0091ab765d54ce643bd82d353c040631
DIFF: https://github.com/llvm/llvm-project/commit/3665da3d0091ab765d54ce643bd82d353c040631.diff
LOG: Re-commit "[clang][Interp] Unify visiting variable declarations"
We often visit the same variable multiple times, e.g. once when checking
its initializer and later when compiling the function. Unify both of
those in visitVarDecl() and do the returning of the value in
visitDecl().
This time, use a VariableScope instead of a DeclScope for local
variables. This way, we don't emit Destroy ops for the local variables
immediately after creating them.
Differential Revision: https://reviews.llvm.org/D136815
Added:
Modified:
clang/lib/AST/Interp/ByteCodeExprGen.cpp
clang/lib/AST/Interp/ByteCodeExprGen.h
clang/lib/AST/Interp/ByteCodeStmtGen.cpp
clang/lib/AST/Interp/ByteCodeStmtGen.h
clang/lib/AST/Interp/Program.cpp
clang/lib/AST/Interp/Program.h
Removed:
################################################################################
diff --git a/clang/lib/AST/Interp/ByteCodeExprGen.cpp b/clang/lib/AST/Interp/ByteCodeExprGen.cpp
index f4bf162a1454c..4bab2897b03b3 100644
--- a/clang/lib/AST/Interp/ByteCodeExprGen.cpp
+++ b/clang/lib/AST/Interp/ByteCodeExprGen.cpp
@@ -802,6 +802,13 @@ unsigned ByteCodeExprGen<Emitter>::allocateLocalPrimitive(DeclTy &&Src,
PrimType Ty,
bool IsConst,
bool IsExtended) {
+ // Make sure we don't accidentally register the same decl twice.
+ if (const auto *VD =
+ dyn_cast_if_present<ValueDecl>(Src.dyn_cast<const Decl *>())) {
+ assert(!P.getGlobal(VD));
+ assert(Locals.find(VD) == Locals.end());
+ }
+
// FIXME: There are cases where Src.is<Expr*>() is wrong, e.g.
// (int){12} in C. Consider using Expr::isTemporaryObject() instead
// or isa<MaterializeTemporaryExpr>().
@@ -817,8 +824,14 @@ unsigned ByteCodeExprGen<Emitter>::allocateLocalPrimitive(DeclTy &&Src,
template <class Emitter>
std::optional<unsigned>
ByteCodeExprGen<Emitter>::allocateLocal(DeclTy &&Src, bool IsExtended) {
- QualType Ty;
+ // Make sure we don't accidentally register the same decl twice.
+ if (const auto *VD =
+ dyn_cast_if_present<ValueDecl>(Src.dyn_cast<const Decl *>())) {
+ assert(!P.getGlobal(VD));
+ assert(Locals.find(VD) == Locals.end());
+ }
+ QualType Ty;
const ValueDecl *Key = nullptr;
const Expr *Init = nullptr;
bool IsTemporary = false;
@@ -1128,41 +1141,86 @@ bool ByteCodeExprGen<Emitter>::visitExpr(const Expr *Exp) {
return this->emitRetValue(Exp);
}
+/// Toplevel visitDecl().
+/// We get here from evaluateAsInitializer().
+/// We need to evaluate the initializer and return its value.
template <class Emitter>
bool ByteCodeExprGen<Emitter>::visitDecl(const VarDecl *VD) {
+ std::optional<PrimType> VarT = classify(VD->getType());
+
+ // Create and initialize the variable.
+ if (!this->visitVarDecl(VD))
+ return false;
+
+ // Get a pointer to the variable
+ if (shouldBeGloballyIndexed(VD)) {
+ auto GlobalIndex = P.getGlobal(VD);
+ assert(GlobalIndex); // visitVarDecl() didn't return false.
+ if (!this->emitGetPtrGlobal(*GlobalIndex, VD))
+ return false;
+ } else {
+ auto Local = Locals.find(VD);
+ assert(Local != Locals.end()); // Same here.
+ if (!this->emitGetPtrLocal(Local->second.Offset, VD))
+ return false;
+ }
+
+ // Return the value
+ if (VarT) {
+ if (!this->emitLoadPop(*VarT, VD))
+ return false;
+
+ return this->emitRet(*VarT, VD);
+ }
+
+ return this->emitRetValue(VD);
+}
+
+template <class Emitter>
+bool ByteCodeExprGen<Emitter>::visitVarDecl(const VarDecl *VD) {
const Expr *Init = VD->getInit();
+ std::optional<PrimType> VarT = classify(VD->getType());
+
+ if (shouldBeGloballyIndexed(VD)) {
+ std::optional<unsigned> GlobalIndex = P.getOrCreateGlobal(VD, Init);
+
+ if (!GlobalIndex)
+ return this->bail(VD);
+
+ assert(Init);
+ {
+ DeclScope<Emitter> LocalScope(this, VD);
- if (std::optional<unsigned> I = P.createGlobal(VD, Init)) {
- if (std::optional<PrimType> T = classify(VD->getType())) {
- {
- // Primitive declarations - compute the value and set it.
- DeclScope<Emitter> LocalScope(this, VD);
- if (!visit(Init))
+ if (VarT) {
+ if (!this->visit(Init))
return false;
+ return this->emitInitGlobal(*VarT, *GlobalIndex, VD);
}
+ return this->visitGlobalInitializer(Init, *GlobalIndex);
+ }
+ } else {
+ VariableScope<Emitter> LocalScope(this);
+ if (VarT) {
+ unsigned Offset = this->allocateLocalPrimitive(
+ VD, *VarT, VD->getType().isConstQualified());
+ if (Init) {
+ // Compile the initializer in its own scope.
+ ExprScope<Emitter> Scope(this);
+ if (!this->visit(Init))
+ return false;
- // If the declaration is global, save the value for later use.
- if (!this->emitDup(*T, VD))
- return false;
- if (!this->emitInitGlobal(*T, *I, VD))
- return false;
- return this->emitRet(*T, VD);
+ return this->emitSetLocal(*VarT, Offset, VD);
+ }
} else {
- {
- // Composite declarations - allocate storage and initialize it.
- DeclScope<Emitter> LocalScope(this, VD);
- if (!visitGlobalInitializer(Init, *I))
- return false;
+ if (std::optional<unsigned> Offset = this->allocateLocal(VD)) {
+ if (Init)
+ return this->visitLocalInitializer(Init, *Offset);
}
-
- // Return a pointer to the global.
- if (!this->emitGetPtrGlobal(*I, VD))
- return false;
- return this->emitRetValue(VD);
}
+ return true;
}
- return this->bail(VD);
+ return false;
}
template <class Emitter>
diff --git a/clang/lib/AST/Interp/ByteCodeExprGen.h b/clang/lib/AST/Interp/ByteCodeExprGen.h
index a8736314c01d0..89782bd87c322 100644
--- a/clang/lib/AST/Interp/ByteCodeExprGen.h
+++ b/clang/lib/AST/Interp/ByteCodeExprGen.h
@@ -132,6 +132,8 @@ class ByteCodeExprGen : public ConstStmtVisitor<ByteCodeExprGen<Emitter>, bool>,
bool visitArrayInitializer(const Expr *Initializer);
/// Compiles a record initializer.
bool visitRecordInitializer(const Expr *Initializer);
+ /// Creates and initializes a variable from the given decl.
+ bool visitVarDecl(const VarDecl *VD);
/// Visits an expression and converts it to a boolean.
bool visitBool(const Expr *E);
@@ -234,6 +236,12 @@ class ByteCodeExprGen : public ConstStmtVisitor<ByteCodeExprGen<Emitter>, bool>,
return T->getAsCXXRecordDecl();
}
+ /// Returns whether we should create a global variable for the
+ /// given VarDecl.
+ bool shouldBeGloballyIndexed(const VarDecl *VD) const {
+ return VD->hasGlobalStorage() || VD->isConstexpr();
+ }
+
protected:
/// Variable to storage mapping.
llvm::DenseMap<const ValueDecl *, Scope::Local> Locals;
@@ -260,6 +268,11 @@ extern template class ByteCodeExprGen<EvalEmitter>;
/// Scope chain managing the variable lifetimes.
template <class Emitter> class VariableScope {
public:
+ VariableScope(ByteCodeExprGen<Emitter> *Ctx)
+ : Ctx(Ctx), Parent(Ctx->VarScope) {
+ Ctx->VarScope = this;
+ }
+
virtual ~VariableScope() { Ctx->VarScope = this->Parent; }
void add(const Scope::Local &Local, bool IsExtended) {
@@ -284,11 +297,6 @@ template <class Emitter> class VariableScope {
VariableScope *getParent() { return Parent; }
protected:
- VariableScope(ByteCodeExprGen<Emitter> *Ctx)
- : Ctx(Ctx), Parent(Ctx->VarScope) {
- Ctx->VarScope = this;
- }
-
/// ByteCodeExprGen instance.
ByteCodeExprGen<Emitter> *Ctx;
/// Link to the parent scope.
diff --git a/clang/lib/AST/Interp/ByteCodeStmtGen.cpp b/clang/lib/AST/Interp/ByteCodeStmtGen.cpp
index 60506a759f45c..af97c57c98b74 100644
--- a/clang/lib/AST/Interp/ByteCodeStmtGen.cpp
+++ b/clang/lib/AST/Interp/ByteCodeStmtGen.cpp
@@ -207,7 +207,7 @@ bool ByteCodeStmtGen<Emitter>::visitDeclStmt(const DeclStmt *DS) {
for (auto *D : DS->decls()) {
// Variable declarator.
if (auto *VD = dyn_cast<VarDecl>(D)) {
- if (!visitVarDecl(VD))
+ if (!this->visitVarDecl(VD))
return false;
continue;
}
@@ -391,41 +391,6 @@ bool ByteCodeStmtGen<Emitter>::visitContinueStmt(const ContinueStmt *S) {
return this->jump(*ContinueLabel);
}
-template <class Emitter>
-bool ByteCodeStmtGen<Emitter>::visitVarDecl(const VarDecl *VD) {
- if (!VD->hasLocalStorage()) {
- // No code generation required.
- return true;
- }
-
- // Integers, pointers, primitives.
- if (std::optional<PrimType> T = this->classify(VD->getType())) {
- const Expr *Init = VD->getInit();
-
- unsigned Offset =
- this->allocateLocalPrimitive(VD, *T, VD->getType().isConstQualified());
- // Compile the initializer in its own scope.
- if (Init) {
- ExprScope<Emitter> Scope(this);
- if (!this->visit(Init))
- return false;
-
- return this->emitSetLocal(*T, Offset, VD);
- }
- return true;
- }
-
- // Composite types - allocate storage and initialize it.
- if (std::optional<unsigned> Offset = this->allocateLocal(VD)) {
- if (!VD->getInit())
- return true;
-
- return this->visitLocalInitializer(VD->getInit(), *Offset);
- }
-
- return this->bail(VD);
-}
-
namespace clang {
namespace interp {
diff --git a/clang/lib/AST/Interp/ByteCodeStmtGen.h b/clang/lib/AST/Interp/ByteCodeStmtGen.h
index 2385c3e55c2f4..829e199f827c6 100644
--- a/clang/lib/AST/Interp/ByteCodeStmtGen.h
+++ b/clang/lib/AST/Interp/ByteCodeStmtGen.h
@@ -63,10 +63,6 @@ class ByteCodeStmtGen final : public ByteCodeExprGen<Emitter> {
bool visitBreakStmt(const BreakStmt *S);
bool visitContinueStmt(const ContinueStmt *S);
- /// Compiles a variable declaration.
- bool visitVarDecl(const VarDecl *VD);
-
-private:
/// Type of the expression returned by the function.
std::optional<PrimType> ReturnType;
diff --git a/clang/lib/AST/Interp/Program.cpp b/clang/lib/AST/Interp/Program.cpp
index 067f6d90b6c11..5305ddd8de189 100644
--- a/clang/lib/AST/Interp/Program.cpp
+++ b/clang/lib/AST/Interp/Program.cpp
@@ -125,11 +125,12 @@ std::optional<unsigned> Program::getGlobal(const ValueDecl *VD) {
return Index;
}
-std::optional<unsigned> Program::getOrCreateGlobal(const ValueDecl *VD) {
+std::optional<unsigned> Program::getOrCreateGlobal(const ValueDecl *VD,
+ const Expr *Init) {
if (auto Idx = getGlobal(VD))
return Idx;
- if (auto Idx = createGlobal(VD, nullptr)) {
+ if (auto Idx = createGlobal(VD, Init)) {
GlobalIndices[VD] = *Idx;
return Idx;
}
@@ -157,6 +158,7 @@ std::optional<unsigned> Program::getOrCreateDummy(const ParmVarDecl *PD) {
std::optional<unsigned> Program::createGlobal(const ValueDecl *VD,
const Expr *Init) {
+ assert(!getGlobal(VD));
bool IsStatic, IsExtern;
if (auto *Var = dyn_cast<VarDecl>(VD)) {
IsStatic = !Var->hasLocalStorage();
diff --git a/clang/lib/AST/Interp/Program.h b/clang/lib/AST/Interp/Program.h
index f49ca6db13e11..5a80dd1ed748e 100644
--- a/clang/lib/AST/Interp/Program.h
+++ b/clang/lib/AST/Interp/Program.h
@@ -79,7 +79,8 @@ class Program final {
std::optional<unsigned> getGlobal(const ValueDecl *VD);
/// Returns or creates a global an creates an index to it.
- std::optional<unsigned> getOrCreateGlobal(const ValueDecl *VD);
+ std::optional<unsigned> getOrCreateGlobal(const ValueDecl *VD,
+ const Expr *Init = nullptr);
/// Returns or creates a dummy value for parameters.
std::optional<unsigned> getOrCreateDummy(const ParmVarDecl *PD);
More information about the cfe-commits
mailing list