Author: Timm Bäder
Date: 2023-01-19T10:46:16+01:00
New Revision: 5b54cf1a2892767fe949826a32d7820732028a38

URL: 
https://github.com/llvm/llvm-project/commit/5b54cf1a2892767fe949826a32d7820732028a38
DIFF: 
https://github.com/llvm/llvm-project/commit/5b54cf1a2892767fe949826a32d7820732028a38.diff

LOG: [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().

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 f4bf162a1454..d0140863c5d7 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,87 @@ 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 {
+    DeclScope<Emitter> LocalScope(this, VD);
+
+    if (VarT) {
+      unsigned Offset = this->allocateLocalPrimitive(
+          VD, *VarT, VD->getType().isConstQualified());
+      // Compile the initializer in its own scope.
+      if (Init) {
+        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 a8736314c01d..4ae70543973e 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;

diff  --git a/clang/lib/AST/Interp/ByteCodeStmtGen.cpp 
b/clang/lib/AST/Interp/ByteCodeStmtGen.cpp
index 60506a759f45..af97c57c98b7 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 2385c3e55c2f..7b6aa56276c8 100644
--- a/clang/lib/AST/Interp/ByteCodeStmtGen.h
+++ b/clang/lib/AST/Interp/ByteCodeStmtGen.h
@@ -64,7 +64,6 @@ class ByteCodeStmtGen final : public ByteCodeExprGen<Emitter> 
{
   bool visitContinueStmt(const ContinueStmt *S);
 
   /// Compiles a variable declaration.
-  bool visitVarDecl(const VarDecl *VD);
 
 private:
   /// Type of the expression returned by the function.

diff  --git a/clang/lib/AST/Interp/Program.cpp 
b/clang/lib/AST/Interp/Program.cpp
index 067f6d90b6c1..5305ddd8de18 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 f49ca6db13e1..5a80dd1ed748 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);


        
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to