From c1d2b983860840b43a5a7c06f961a45e91e176e6 Mon Sep 17 00:00:00 2001 From: Wilson Snyder Date: Tue, 30 Dec 2008 17:11:25 -0500 Subject: [PATCH] Fix wrong result for read of delayed FSM signal, bug46. --- Changes | 2 + src/V3Table.cpp | 69 +++++++++++---- test_regress/t/t_table_fsm.pl | 17 ++++ test_regress/t/t_table_fsm.v | 159 ++++++++++++++++++++++++++++++++++ 4 files changed, 228 insertions(+), 19 deletions(-) create mode 100755 test_regress/t/t_table_fsm.pl create mode 100644 test_regress/t/t_table_fsm.v diff --git a/Changes b/Changes index de94c2e6c..1f564a65b 100644 --- a/Changes +++ b/Changes @@ -37,6 +37,8 @@ indicates the contributor was also the author of the fix; Thanks! **** Fix internal error on "output x; reg x = y;". +**** Fix wrong result for read of delayed FSM signal, bug46. [Rodney Sinclair] + * Verilator 3.681 2008/11/12 *** Add SystemVerilog unique and priority case. diff --git a/src/V3Table.cpp b/src/V3Table.cpp index 693d80092..fb283137a 100644 --- a/src/V3Table.cpp +++ b/src/V3Table.cpp @@ -74,8 +74,8 @@ private: // Checking: // AstVarScope::user1() -> VarUsage. Set true to indicate tracking as lvalue/rvalue // Simulating: - // AstVarScope::user3() -> V3Number*. Value of variable or node - // AstVarScope::user4() -> V3Number*. Last value output was set to + // AstVarScope::user3() -> V3Number*. Input value of variable or node (and output for non-delayed assignments) + // AstVarScope::user4() -> V3Number*. Output value of variable (delayed assignments) enum VarUsage { VU_NONE=0, VU_LV=1, VU_RV=2, VU_LVDLY=4 }; @@ -113,42 +113,67 @@ public: int dataCount() const { return m_dataCount; } // Simulation METHODS +private: + V3Number* allocNumber(AstNode* nodep, uint32_t value) { + // Save time - kept a list of allocated but unused V3Numbers + // It would be more efficient to do this by size, but the extra accounting + // slows things down more than we gain. + V3Number* nump; + if (!m_numFreeps.empty()) { + //UINFO(7,"Num Reuse "<width()<width(nodep->width()); + nump->fileline(nodep->fileline()); + nump->setLong(value); // We do support more than 32 bit numbers, just valuep=0 in that case + } else { + //UINFO(7,"Num New "<width()<fileline(), nodep->width(), value); + m_numAllps.push_back(nump); + } + return nump; + } public: V3Number* newNumber(AstNode* nodep, uint32_t value=0) { // Set a constant value for this node if (!nodep->user3p()) { - // Save time - kept a list of allocated but unused V3Numbers - // It would be more efficient to do this by size, but the extra accounting - // slows things down more than we gain. - V3Number* nump; - if (!m_numFreeps.empty()) { - //UINFO(7,"Num Reuse "<width()<width(nodep->width()); - nump->fileline(nodep->fileline()); - nump->setLong(value); // We do support more than 32 bit numbers, just valuep=0 in that case - } else { - //UINFO(7,"Num New "<width()<fileline(), nodep->width(), value); - m_numAllps.push_back(nump); - } + V3Number* nump = allocNumber(nodep, value); setNumber(nodep, nump); } return (fetchNumber(nodep)); } + V3Number* newOutNumber(AstNode* nodep, uint32_t value=0) { + // Set a constant value for this node + if (!nodep->user4p()) { + V3Number* nump = allocNumber(nodep, value); + setOutNumber(nodep, nump); + } + return (fetchOutNumber(nodep)); + } V3Number* fetchNumberNull(AstNode* nodep) { return ((V3Number*)nodep->user3p()); } + V3Number* fetchOutNumberNull(AstNode* nodep) { + return ((V3Number*)nodep->user4p()); + } V3Number* fetchNumber(AstNode* nodep) { V3Number* nump = fetchNumberNull(nodep); if (!nump) nodep->v3fatalSrc("No value found for node."); return nump; } + V3Number* fetchOutNumber(AstNode* nodep) { + V3Number* nump = fetchOutNumberNull(nodep); + if (!nump) nodep->v3fatalSrc("No value found for node."); + return nump; + } private: void setNumber(AstNode* nodep, const V3Number* nump) { UINFO(9," set num "<<*nump<<" on "<user3p((AstNUser*)nump); } + void setOutNumber(AstNode* nodep, const V3Number* nump) { + UINFO(9," set num "<<*nump<<" on "<user4p((AstNUser*)nump); + } void checkNodeInfo(AstNode* nodep) { m_instrCount += nodep->instrCount(); @@ -298,7 +323,13 @@ private: else if (!m_checking) { nodep->rhsp()->iterateAndNext(*this); AstVarScope* vscp = nodep->lhsp()->castVarRef()->varScopep(); - setNumber(vscp, fetchNumber(nodep->rhsp())); + if (nodep->castAssignDly()) { + // Don't do setNumber, as value isn't visible to new statements + setOutNumber(vscp, fetchNumber(nodep->rhsp())); + } else { + setNumber(vscp, fetchNumber(nodep->rhsp())); + setOutNumber(vscp, fetchNumber(nodep->rhsp())); + } } m_inDlyAssign = false; } @@ -593,7 +624,7 @@ private: V3Number outputChgMask (nodep->fileline(), m_outVarps.size(), 0); for (deque::iterator it = m_outVarps.begin(); it!=m_outVarps.end(); ++it) { AstVarScope* outvscp = *it; - V3Number* outnump = simvis.fetchNumberNull(outvscp); + V3Number* outnump = simvis.fetchOutNumberNull(outvscp); AstNode* setp; if (!outnump) { UINFO(8," Output "<name()<<" never set\n"); diff --git a/test_regress/t/t_table_fsm.pl b/test_regress/t/t_table_fsm.pl new file mode 100755 index 000000000..cdae47250 --- /dev/null +++ b/test_regress/t/t_table_fsm.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 +# General Public License or the Perl Artistic License. + +compile ( + ); + +execute ( + check_finished=>1, + ); + +ok(1); +1; diff --git a/test_regress/t/t_table_fsm.v b/test_regress/t/t_table_fsm.v new file mode 100644 index 000000000..1ac617f8a --- /dev/null +++ b/test_regress/t/t_table_fsm.v @@ -0,0 +1,159 @@ +// DESCRIPTION: Verilator: Verilog Test module +// This file ONLY is placed into the Public Domain, for any use, +// without warranty, 2008 by Wilson Snyder. + +module t (/*AUTOARG*/ + // Inputs + clk + ); + input clk; + + integer cyc=0; + reg [63:0] crc; + reg [63:0] sum; + reg reset; + + /*AUTOWIRE*/ + // Beginning of automatic wires (for undeclared instantiated-module outputs) + wire myevent; // From test of Test.v + wire myevent_pending; // From test of Test.v + wire [1:0] state; // From test of Test.v + // End of automatics + + Test test (/*AUTOINST*/ + // Outputs + .state (state[1:0]), + .myevent (myevent), + .myevent_pending (myevent_pending), + // Inputs + .clk (clk), + .reset (reset)); + + // Aggregate outputs into a single result vector + wire [63:0] result = {60'h0, myevent_pending,myevent,state}; + + // Test loop + always @ (posedge clk) begin +`ifdef TEST_VERBOSE + $write("[%0t] cyc==%0d crc=%x result=%x me=%0x mep=%x\n",$time, cyc, crc, result, myevent, myevent_pending); +`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]}; + reset <= (cyc<2); + if (cyc==0) begin + // Setup + crc <= 64'h5aef0c8d_d70a4497; + 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'h4e93a74bd97b25ef + if (sum !== `EXPECTED_SUM) $stop; + $write("*-* All Finished *-*\n"); + $finish; + end + end + +endmodule + +module Test (/*AUTOARG*/ + // Outputs + state, myevent, myevent_pending, + // Inputs + clk, reset + ); + input clk; + input reset; + output [1:0] state; + output myevent; + output myevent_pending; + + reg [5:0] count = 0; + always @ (posedge clk) + if (reset) count <= 0; + else count <= count + 1; + + reg myevent = 1'b0; + always @ (posedge clk) + myevent <= (count == 6'd27); + + reg myevent_done; + reg hickup_ready; + reg hickup_done; + + localparam STATE_ZERO = 0; + localparam STATE_ONE = 1; + localparam STATE_TWO = 2; + + reg [1:0] state = STATE_ZERO; + reg state_start_myevent = 1'b0; + reg state_start_hickup = 1'b0; + reg myevent_pending = 1'b0; + always @ (posedge clk) begin + state <= state; + myevent_pending <= myevent_pending || myevent; + state_start_myevent <= 1'b0; + state_start_hickup <= 1'b0; + case (state) + STATE_ZERO: + if (myevent_pending) begin + state <= STATE_ONE; + myevent_pending <= 1'b0; + state_start_myevent <= 1'b1; + end else if (hickup_ready) begin + state <= STATE_TWO; + state_start_hickup <= 1'b1; + end + + STATE_ONE: + if (myevent_done) + state <= STATE_ZERO; + + STATE_TWO: + if (hickup_done) + state <= STATE_ZERO; + + default: + ; /* do nothing */ + endcase + end + + reg [3:0] myevent_count = 0; + always @ (posedge clk) + if (state_start_myevent) + myevent_count <= 9; + else if (myevent_count > 0) + myevent_count <= myevent_count - 1; + + initial myevent_done = 1'b0; + always @ (posedge clk) + myevent_done <= (myevent_count == 0); + + reg [4:0] hickup_backlog = 2; + always @ (posedge clk) + if (state_start_myevent) + hickup_backlog <= hickup_backlog - 1; + else if (state_start_hickup) + hickup_backlog <= hickup_backlog + 1; + + initial hickup_ready = 1'b1; + always @ (posedge clk) + hickup_ready <= (hickup_backlog < 3); + + reg [3:0] hickup_count = 0; + always @ (posedge clk) + if (state_start_hickup) + hickup_count <= 10; + else if (hickup_count > 0) + hickup_count <= hickup_count - 1; + + initial hickup_done = 1'b0; + always @ (posedge clk) + hickup_done <= (hickup_count == 1); + +endmodule