From 896b18e024d8a4b6263e3ebe4e1362d327f47f72 Mon Sep 17 00:00:00 2001
From: Geza Lore <gezalore@gmail.com>
Date: Fri, 2 Jul 2021 16:55:57 +0100
Subject: [PATCH] Emit function locals in the place they appear in the tree

Do not sort and hoist function local variables to the top of the
function definition. The stack layout of automatic variables is not
defined by C so the compilers can lay these out optimally. Simplifies
internals for follow on work. Effect on model performance is neutral to
very slight improvement, so we do not seem to be loosing anything.
---
 src/V3EmitCFunc.cpp |  5 ++---
 src/V3EmitCFunc.h   | 35 ++++++++++++++++++++++-------------
 2 files changed, 24 insertions(+), 16 deletions(-)

diff --git a/src/V3EmitCFunc.cpp b/src/V3EmitCFunc.cpp
index 5a5613981..14a82ea82 100644
--- a/src/V3EmitCFunc.cpp
+++ b/src/V3EmitCFunc.cpp
@@ -544,7 +544,6 @@ void EmitCFunc::emitVarList(AstNode* firstp, EisWhich which, const string& prefi
                     doit = (varp->isParam() && !VN_IS(varp->valuep(), Const));
                     break;
                 case EVL_CLASS_ALL: doit = true; break;
-                case EVL_FUNC_ALL: doit = true; break;
                 default: v3fatalSrc("Bad Case");
                 }
                 if (varp->isStatic() ? !isstatic : isstatic) doit = false;
@@ -574,8 +573,8 @@ void EmitCFunc::emitVarList(AstNode* firstp, EisWhich which, const string& prefi
                            && !varp->isSc()  // Aggregates can't be anon
                            && (varp->basicp()
                                && !varp->basicp()->isOpaque())  // Aggregates can't be anon
-                           && which != EVL_FUNC_ALL);  // Anon not legal in funcs, and gcc
-                                                       // bug free there anyhow
+                        );
+
                     if (anonOk) {
                         varAnonMap[sortbytes].push_back(varp);
                     } else {
diff --git a/src/V3EmitCFunc.h b/src/V3EmitCFunc.h
index 38f18a00e..534739694 100644
--- a/src/V3EmitCFunc.h
+++ b/src/V3EmitCFunc.h
@@ -130,6 +130,7 @@ protected:
     EmitCLazyDecls m_lazyDecls;  // Visitor for emitting lazy declarations
     bool m_useSelfForThis = false;  // Replace "this" with "vlSelf"
     AstNodeModule* m_modp = nullptr;  // Current module being emitted
+    AstCFunc* m_cfuncp = nullptr;  // Current function being emitted
 
 public:
     // METHODS
@@ -159,8 +160,7 @@ public:
         EVL_CLASS_SIG,
         EVL_CLASS_TEMP,
         EVL_CLASS_PAR,
-        EVL_CLASS_ALL,
-        EVL_FUNC_ALL
+        EVL_CLASS_ALL
     };
     void emitVarList(AstNode* firstp, EisWhich which, const string& prefixIfImp, string& sectionr);
     static void emitVarSort(const VarSortMap& vmap, VarVec* sortedp);
@@ -203,6 +203,8 @@ public:
     // VISITORS
     virtual void visit(AstCFunc* nodep) override {
         VL_RESTORER(m_useSelfForThis);
+        VL_RESTORER(m_cfuncp);
+        m_cfuncp = nodep;
 
         m_blkChangeDetVec.clear();
 
@@ -238,25 +240,28 @@ public:
         puts(nodep->isLoose() ? "__" : "::");
         puts(nodep->nameProtect() + "\\n\"); );\n");
 
-        if (nodep->initsp()) putsDecoration("// Variables\n");
         for (AstNode* subnodep = nodep->argsp(); subnodep; subnodep = subnodep->nextp()) {
             if (AstVar* varp = VN_CAST(subnodep, Var)) {
                 if (varp->isFuncReturn()) emitVarDecl(varp, "");
             }
         }
-        string section;
-        emitVarList(nodep->initsp(), EVL_FUNC_ALL, "", section /*ref*/);
-        emitVarList(nodep->stmtsp(), EVL_FUNC_ALL, "", section /*ref*/);
 
-        iterateAndNextNull(nodep->initsp());
+        if (nodep->initsp()) {
+            putsDecoration("// Init\n");
+            iterateAndNextNull(nodep->initsp());
+        }
+
+        if (nodep->stmtsp()) {
+            putsDecoration("// Body\n");
+            iterateAndNextNull(nodep->stmtsp());
+        }
 
-        if (nodep->stmtsp()) putsDecoration("// Body\n");
-        iterateAndNextNull(nodep->stmtsp());
         if (!m_blkChangeDetVec.empty()) emitChangeDet();
 
-        if (nodep->finalsp()) putsDecoration("// Final\n");
-        iterateAndNextNull(nodep->finalsp());
-        //
+        if (nodep->finalsp()) {
+            putsDecoration("// Final\n");
+            iterateAndNextNull(nodep->finalsp());
+        }
 
         if (!m_blkChangeDetVec.empty()) puts("return __req;\n");
 
@@ -264,6 +269,11 @@ public:
         if (nodep->ifdef() != "") puts("#endif  // " + nodep->ifdef() + "\n");
     }
 
+    virtual void visit(AstVar* nodep) override {
+        UASSERT_OBJ(m_cfuncp, nodep, "Cannot emit non-local variable");
+        emitVarDecl(nodep, "");
+    }
+
     virtual void visit(AstNodeAssign* nodep) override {
         bool paren = true;
         bool decind = false;
@@ -1221,7 +1231,6 @@ public:
     virtual void visit(AstTypedef*) override {}
     virtual void visit(AstPragma*) override {}
     virtual void visit(AstCell*) override {}  // Handled outside the Visit class
-    virtual void visit(AstVar*) override {}  // Handled outside the Visit class
     virtual void visit(AstNodeText*) override {}  // Handled outside the Visit class
     virtual void visit(AstCFile*) override {}  // Handled outside the Visit class
     virtual void visit(AstCellInline*) override {}  // Handled outside visit (in EmitCSyms)