From 847dbbbaf011973dc5fc0802265d8bdf061c0ec0 Mon Sep 17 00:00:00 2001
From: Wilson Snyder <wsnyder@wsnyder.org>
Date: Tue, 21 Aug 2018 18:09:40 -0400
Subject: [PATCH] Fix function inlining inside certain while loops, bug1330.

---
 Changes                        |  4 +++-
 src/V3Task.cpp                 | 15 +++++++++++----
 test_regress/t/t_func_while.pl | 17 +++++++++++++++++
 test_regress/t/t_func_while.v  | 32 ++++++++++++++++++++++++++++++++
 4 files changed, 63 insertions(+), 5 deletions(-)
 create mode 100755 test_regress/t/t_func_while.pl
 create mode 100644 test_regress/t/t_func_while.v

diff --git a/Changes b/Changes
index 3406f20ae..cf8656acb 100644
--- a/Changes
+++ b/Changes
@@ -6,13 +6,15 @@ The contributors that suggested a given feature are shown in []. Thanks!
 
 ****  Add OBJCACHE envvar support to examples and generated Makefiles.
 
+****  Change MODDUP errors to warnings, msg2588. [Marshal Qiao]
+
 ****  Fix define argument stringification (`"), broke since 3.914. [Joe DErrico]
 
 ****  Fix to ignore Unicode UTF-8 BOM sequences, msg2576. [HyungKi Jeong]
 
 ****  Fix std:: build error, bug1322.
 
-****  Change MODDUP errors to warnings, msg2588. [Marshal Qiao]
+****  Fix function inlining inside certain while loops, bug1330. [Julien Margetts]
 
 
 * Verilator 3.924 2018-06-12
diff --git a/src/V3Task.cpp b/src/V3Task.cpp
index 9832b2d72..09a275bfa 100644
--- a/src/V3Task.cpp
+++ b/src/V3Task.cpp
@@ -1081,10 +1081,12 @@ private:
 	m_insMode = prevInsMode;
 	m_insStmtp = prevInsStmtp;
     }
-    void insertBeforeStmt(AstNode* nodep, AstNode* newp) {
+    AstNode* insertBeforeStmt(AstNode* nodep, AstNode* newp) {
+	// Return node that must be visited, if any
 	// See also AstNode::addBeforeStmt; this predates that function
 	if (debug()>=9) { nodep->dumpTree(cout,"-newstmt:"); }
 	if (!m_insStmtp) nodep->v3fatalSrc("Function not underneath a statement");
+	AstNode* visitp = NULL;
 	if (m_insMode == IM_BEFORE) {
 	    // Add the whole thing before insertAt
 	    UINFO(5,"     IM_Before  "<<m_insStmtp<<endl);
@@ -1092,20 +1094,22 @@ private:
 	    m_insStmtp->addHereThisAsNext(newp);
 	}
 	else if (m_insMode == IM_AFTER) {
-	    UINFO(5,"     IM_After   "<<m_insStmtp);
+	    UINFO(5,"     IM_After   "<<m_insStmtp<<endl);
 	    m_insStmtp->addNextHere(newp);
 	}
 	else if (m_insMode == IM_WHILE_PRECOND) {
-	    UINFO(5,"     IM_While_Precond "<<m_insStmtp);
+	    UINFO(5,"     IM_While_Precond "<<m_insStmtp<<endl);
 	    AstWhile* whilep = m_insStmtp->castWhile();
 	    if (!whilep) nodep->v3fatalSrc("Insert should be under WHILE");
 	    whilep->addPrecondsp(newp);
+	    visitp = newp;
 	}
 	else {
 	    nodep->v3fatalSrc("Unknown InsertMode");
 	}
 	m_insMode = IM_AFTER;
 	m_insStmtp = newp;
+	return visitp;
     }
 
     // VISITORS
@@ -1150,12 +1154,13 @@ private:
 	    beginp = createInlinedFTask(nodep, namePrefix, outvscp);
 	}
 	// Replace the ref
+	AstNode* visitp = NULL;
 	if (nodep->castFuncRef()) {
 	    if (!nodep->taskp()->isFunction()) nodep->v3fatalSrc("func reference to non-function");
 	    AstVarRef* outrefp = new AstVarRef (nodep->fileline(), outvscp, false);
 	    nodep->replaceWith(outrefp);
 	    // Insert new statements
-	    insertBeforeStmt(nodep, beginp);
+	    visitp = insertBeforeStmt(nodep, beginp);
 	} else {
 	    // outvscp maybe non-NULL if calling a function in a taskref,
 	    // but if so we want to simply ignore the function result
@@ -1164,6 +1169,8 @@ private:
 	// Cleanup
 	nodep->deleteTree(); VL_DANGLING(nodep);
 	UINFO(4,"  FTask REF Done.\n");
+	// Visit nodes that normal iteration won't find
+	if (visitp) visitp->iterateAndNext(*this);
     }
     virtual void visit(AstNodeFTask* nodep) {
 	UINFO(4," Inline   "<<nodep<<endl);
diff --git a/test_regress/t/t_func_while.pl b/test_regress/t/t_func_while.pl
new file mode 100755
index 000000000..b660b3660
--- /dev/null
+++ b/test_regress/t/t_func_while.pl
@@ -0,0 +1,17 @@
+#!/usr/bin/perl
+if (!$::Driver) { use FindBin; exec("$FindBin::Bin/bootstrap.pl", @ARGV, $0); die; }
+# DESCRIPTION: Verilator: Verilog Test driver/expect definition
+#
+# Copyright 2003 by Wilson Snyder. This program is free software; you can
+# redistribute it and/or modify it under the terms of either the GNU
+# Lesser General Public License Version 3 or the Perl Artistic License
+# Version 2.0.
+
+scenarios(simulator => 1);
+
+compile(
+    verilator_flags2 => ["--trace"],
+    );
+
+ok(1);
+1;
diff --git a/test_regress/t/t_func_while.v b/test_regress/t/t_func_while.v
new file mode 100644
index 000000000..1591d058f
--- /dev/null
+++ b/test_regress/t/t_func_while.v
@@ -0,0 +1,32 @@
+// DESCRIPTION: Verilator: Verilog Test module
+//
+// This file ONLY is placed into the Public Domain, for any use,
+// without warranty, 2018 by Julien Margetts.
+
+module t #(parameter sz = 4096)
+   (
+    input wire                     clk,
+    output reg [tdw(sz)-1:0] data
+    );
+
+   // bug1330
+   function integer clog2(input integer value);
+      integer tmp;
+      tmp   = value-1;
+      clog2 = 0;
+      for (clog2=0; (tmp>0) && (clog2<32); clog2=clog2+1)
+        tmp = tmp>>1;
+   endfunction
+
+   function integer tdw(input integer sz);
+      tdw = clog2(sz);
+   endfunction
+
+   integer b;
+
+   always @(posedge clk)
+     for (b=0; b<tdw(sz); b=b+1)
+       if ((data[b] === 1'bx))
+         $display("WARNING: %1t Writing X's to tag RAM [%m]", $time);
+
+endmodule