Fix bad result with if-else-return optimization, bug420.

Changes

@@ -14,6 +14,8 @@ indicates the contributor was also the author of the fix; Thanks!
 
 **** Indicate 'exiting due to errors' if errors, not warnings. [Ruben Diez]
 
+**** Fix bad result with if-else-return optimization, bug420. [Alex Solomatnikov]
+
 **** Fix reporting not found modules if generate-off, bug403. [Jeremy Bennett]
 
 

src/V3Split.cpp

@@ -229,6 +229,7 @@ private:
 
     // STATE
     bool		m_reorder;	// Reorder statements vs. just splitting
+    string		m_noReorderWhy;	// Reason we can't reorder
     VStack		m_stmtStackps;	// Current statements being tracked
     SplitPliVertex*	m_pliVertexp;	// Element specifying PLI ordering
     V3Graph		m_graph;	// Scoreboard of var usages/dependencies
@@ -249,6 +250,7 @@ private:
 	m_graph.clear();
 	m_stmtStackps.clear();
 	m_pliVertexp = NULL;
+	m_noReorderWhy = "";
 	AstNode::user1ClearTree();
 	AstNode::user2ClearTree();
 	AstNode::user3ClearTree();
@@ -336,6 +338,7 @@ private:
 	}
 
 	// Weak coloring to determine what needs to remain in order
+	// This follows all step-relevant edges excluding PostEdges, which are done later
 	m_graph.weaklyConnected(&SplitEdge::followScoreboard);
 
 	// Add hard orderings between all nodes of same color, in the order they appeared
@@ -445,14 +448,18 @@ private:
 	    UINFO(9,"  processBlock "<<nodep<<endl);
 	    // Process block and followers
 	    scanBlock(nodep);
-	    // Reorder statements in this block
-	    cleanupBlockGraph(nodep);
-	    reorderBlock(nodep);
-	    // Delete old vertexes and edges only applying to this block
-	    while (firstp->backp()->nextp()==firstp) firstp = firstp->backp();  // Walk back to first in list
-	    for (AstNode* nextp=firstp; nextp; nextp=nextp->nextp()) {
-		SplitLogicVertex* vvertexp = (SplitLogicVertex*)nextp->user3p();
-		vvertexp->unlinkDelete(&m_graph);
+	    if (m_noReorderWhy != "") {  // Jump or something nasty
+		UINFO(9,"  NoReorderBlock because "<<m_noReorderWhy<<endl);
+	    } else {
+		// Reorder statements in this block
+		cleanupBlockGraph(nodep);
+		reorderBlock(nodep);
+		// Delete old vertexes and edges only applying to this block
+		while (firstp->backp()->nextp()==firstp) firstp = firstp->backp();  // Walk back to first in list
+		for (AstNode* nextp=firstp; nextp; nextp=nextp->nextp()) {
+		    SplitLogicVertex* vvertexp = (SplitLogicVertex*)nextp->user3p();
+		    vvertexp->unlinkDelete(&m_graph);
+		}
 	    }
 	}
 	// Again, nodep may no longer be first.
@@ -534,6 +541,15 @@ private:
 	    }
 	}
     }
+    virtual void visit(AstJumpGo* nodep, AstNUser*) {
+	// Jumps will disable reordering at all levels
+	// This is overly pessimistic; we could treat jumps as barriers, and
+	// reorder everything between jumps/labels, however jumps are rare
+	// in always, so the performance gain probably isn't worth the work.
+	UINFO(9,"         NoReordering "<<nodep<<endl);
+	m_noReorderWhy = "JumpGo";
+	nodep->iterateChildren(*this);
+    }
 
     //--------------------
     // Default

test_regress/t/t_func_return.pl

@@ -0,0 +1,18 @@
+#!/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.
+
+compile (
+	 );
+
+execute (
+	 check_finished=>1,
+     );
+
+ok(1);
+1;

test_regress/t/t_func_return.v

@@ -0,0 +1,66 @@
+// DESCRIPTION: Verilator: Verilog Test module
+//
+// This file ONLY is placed into the Public Domain, for any use,
+// without warranty, 2011 by Wilson Snyder.
+
+// bug420
+typedef logic [7-1:0] wb_ind_t;
+typedef logic [7-1:0] id_t;
+
+module t (/*AUTOARG*/
+   // Inputs
+   clk
+   );
+   input clk;
+
+   integer 	cyc=0;
+   reg [63:0] 	crc;
+   reg [63:0] 	sum;
+
+   // Take CRC data and apply to testblock inputs
+   wire [31:0]  in = crc[31:0];
+
+   /*AUTOWIRE*/
+
+   wire [6:0] out = line_wb_ind( in[6:0] );
+
+   // Aggregate outputs into a single result vector
+   wire [63:0] result = {57'h0, out};
+
+   // Test loop
+   always @ (posedge clk) begin
+//`ifdef TEST_VERBOSE
+      $write("[%0t] cyc==%0d crc=%x result=%x\n",$time, cyc, crc, result);
+//`endif
+      cyc <= cyc + 1;
+      crc <= {crc[62:0], crc[63]^crc[2]^crc[0]};
+      sum <= result ^ {sum[62:0],sum[63]^sum[2]^sum[0]};
+      if (cyc==0) begin
+	 // Setup
+	 crc <= 64'h5aef0c8d_d70a4497;
+	 sum <= 64'h0;
+      end
+      else if (cyc<10) begin
+	 sum <= 64'h0;
+      end
+      else if (cyc<90) begin
+      end
+      else if (cyc==99) begin
+	 $write("[%0t] cyc==%0d crc=%x sum=%x\n",$time, cyc, crc, sum);
+	 if (crc !== 64'hc77bb9b3784ea091) $stop;
+	 // What checksum will we end up with (above print should match)
+`define EXPECTED_SUM 64'hc918fa0aa882a206
+	 if (sum !== `EXPECTED_SUM) $stop;
+	 $write("*-* All Finished *-*\n");
+	 $finish;
+      end
+   end
+
+   function wb_ind_t line_wb_ind( id_t id );
+      if( id[$bits(id_t)-1] == 0 )
+        return {2'b00, id[$bits(wb_ind_t)-3:0]};
+      else
+        return {2'b01, id[$bits(wb_ind_t)-3:0]};
+   endfunction // line_wb_ind
+
+endmodule