diff --git a/Changes b/Changes index ff7852b0c..3fd4db1a0 100644 --- a/Changes +++ b/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] diff --git a/src/V3Split.cpp b/src/V3Split.cpp index eb0f57875..afa2b8c50 100644 --- a/src/V3Split.cpp +++ b/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 "<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 "<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 "<iterateChildren(*this); + } //-------------------- // Default diff --git a/test_regress/t/t_func_return.pl b/test_regress/t/t_func_return.pl new file mode 100755 index 000000000..7058e622f --- /dev/null +++ b/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; diff --git a/test_regress/t/t_func_return.v b/test_regress/t/t_func_return.v new file mode 100644 index 000000000..d7026e6ed --- /dev/null +++ b/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