Fftram fix

rtl/fftram.vp

@@ -144,58 +144,34 @@ module `mname`
    logic [`$rbits`] `$sname`_ix;   logic [`$op_width-1`:0] `$sname`_rd_data;   logic [`$op_width-1`:0] `$sname`_wr_data;   logic `$sname`_ez;   logic `$sname`_wz;
    //; }
 
-
-   //; # no longer optional i guess, everyone needs cycle_num...right? FIXME
-   //; # if (($PRECOMPUTED_BYPASS == 0) && ($n_sram_ports == 1)) {
-   //; if (1 == 1) {
-   // (For now) BOTF: must duplicate fftctl's 'cycle_num' signal so we
-   // can conservatively BYPASS ALL FINAL-CYCLE WRITES in a each stage.
-   // FIXME Really shouldn't bypass final-stage writes!!!
-   // FIXME Later, can just bring 'cycle_num' (below) in from fftctl as a signal.
-   // FIXME Or something smarter.
-   // Need to count from 0 to [log2(npoints) x (npoints/2) / (nbutts)]
-   // plus at least one beyond that.
    logic [`$nbits_cycle_num-1`:0] cycle_num;
-//; if (($PRECOMPUTED_BYPASS == 0) && ($n_sram_ports == 1)) {    # BOTF
+
+   //; if (($PRECOMPUTED_BYPASS == 0) && ($n_sram_ports == 1)) {    # BOTF
+   //;
+   //    Trying something new for bypasses: BOTF (bypass on the fly)
+   //    (For now) BOTF: must duplicate fftctl's 'cycle_num' signal so we
+   //    can conservatively BYPASS ALL FINAL-CYCLE WRITES in a each stage.
+   //    FIXME Really shouldn't bypass final-stage writes!!!
+   //    FIXME Later, can just bring 'cycle_num' (below) in from fftctl as a signal.
+   //    FIXME Or something smarter.
+   //    Need to count from 0 to [log2(npoints) x (npoints/2) / (nbutts)]
+   //    plus at least one beyond that.
    logic bypass_time;              // Bypass_time==1 when cycle_time says it is time to bypass.
-//; }
-   //; } else {
-   // Placeholder for optional BOTF signals, unused in this configuration (cycle_num).
-   //; }
 
-   //; if (($PRECOMPUTED_BYPASS == 0) && ($n_sram_ports == 1)) {
-   // E.g. SRAM001_active=1 during any cycle that we're accessing SRAM001 (BOTF).
-   //; for (my $i=0; $i<$nbanks; $i++) {
-   //;    my $sname = sprintf("SRAM%03d", $i);  # "000", "001", ... "015" etc.
+   //    BOTF: E.g. SRAM001_active=1 during any cycle that we're accessing SRAM001
+   //;   for (my $i=0; $i<$nbanks; $i++) {
+   //;     my $sname = sprintf("SRAM%03d", $i);  # "000", "001", ... "015" etc.
    logic `$sname`_active;
-   //;     }
-   //; } else {
-   // Placeholder for optional BOTF signals, unused in this configuration (SRAM003_active).
-   //; }
-
-//;#   //; if (($PRECOMPUTED_BYPASS == 0) && ($n_sram_ports == 1)) {
-//;#   // Trying something new for bypasses: BOTF (bypass on the fly)
-//;#   //; # logic SRAM000_wants_to_read_then_write;
-//;#   //; # logic SRAM001_wants_to_read_then_write;
-//;#   //; # logic SRAM002_wants_to_read_then_write;
-//;#   //; # ...
-//;#   //;     for (my $i=0; $i<$nbanks; $i++) {
-//;#   //;         my $sname = sprintf("SRAM%03d", $i);  # "000", "001", ... "015" etc.
-//;#   logic `$sname`_wants_to_read_then_write; 
-//;#   //;     }
-//;#   //; } else {
-//;#   // Placeholder for optional BOTF signals, unused in this configuration.
-//;#   //; }
-
-   //; if (($PRECOMPUTED_BYPASS == 0) && ($n_sram_ports == 1)) {
-   // BOTF: At end of each stage, there is possibility of read/write
-   // conflict where one butterfly wants to write to the (single ported)
-   // SRAM at the same time another one wants to read from the SRAM.
-   // When this happens, bypass the write data to the 'bypassed_data' buffer.
-   //; my $nsrams = $nbanks;
-   //; my $r1 = sprintf("%-6s", "");
-   //; my $r2 = sprintf("%-6s", "[$rbits]");
-   //; my $r3 = sprintf("%-6s", "[$obits]");
+   //;   }
+   //;
+   //    BOTF: At end of each stage, there is possibility of read/write
+   //    conflict where one butterfly wants to write to the (single ported)
+   //    SRAM at the same time another one wants to read from the SRAM.
+   //    When this happens, bypass the write data to the 'bypassed_data' buffer.
+   //;   my $nsrams = $nbanks;
+   //;   my $r1 = sprintf("%-6s", "");
+   //;   my $r2 = sprintf("%-6s", "[$rbits]");
+   //;   my $r3 = sprintf("%-6s", "[$obits]");
    //; 
    logic `$r1` bypass_valid[0:`$nsrams-1`];  // True (1) iff bypassed_data is valid
    logic `$r2` bypassed_rnum[0:`$nsrams-1`]; // Use target row number (index) of valid data as an ID.
@@ -249,45 +225,22 @@ module `mname`
    // LOOK MA NO BYPASS BUFFER
    //; }
 
-//;#   //; if (($PRECOMPUTED_BYPASS == 0) && ($n_sram_ports == 1)) {
-//;#   // Initialize BOTF (bypass on the fly) signals
-//;#   //; # SRAM000_wants_to_read_then_write <= 0; 
-//;#   //; # SRAM001_wants_to_read_then_write <= 0; 
-//;#   //; # SRAM002_wants_to_read_then_write <= 0; 
-//;#   //; # ...
-//;#   initial begin
-//;#   //; for (my $i=0; $i<$nbanks; $i++) {
-//;#   //;     my $sname = sprintf("SRAM%03d", $i);  # "000", "001", ... "015" etc.
-//;#      `$sname`_wants_to_read_then_write <= 0; 
-//;#   //; }
-//;#   end
-//;#   //; } else {
-//;#   // Placeholder for initialization of optional BOTF signals, unused in this configuration (sram_wants...).
-//;#   //; }
-
-   //; # FIXME cycle_num no longer optional i guess...
-   //; # if (($PRECOMPUTED_BYPASS == 0) && ($n_sram_ports == 1)) {
-   //; if (1 == 1) {
-   // Initialize BOTF (bypass on the fly) signals (cycle_num)
    initial cycle_num = `$nbits_cycle_num`'b0;
-   //; } else {
-   // Placeholder for initialization of optional BOTF signals, unused in this configuration (cycle_num).
-   //; }
 
-   //; if (($PRECOMPUTED_BYPASS == 0) && ($n_sram_ports == 1)) {
-   // Initialize BOTF (bypass on the fly) signals
-   // bypass_valid:  True (1) iff bypassed_data is valid
-   // bypassed_rnum: Use target row number (index) of valid data as an ID.
-   // bypassed_data: Valid data lives here instead of 'rnum' in SRAM.
-   //; # ...
-   // 'x means "set all bits to x", or so I'm told.
+   //; if (($PRECOMPUTED_BYPASS == 0) && ($n_sram_ports == 1)) {    # BOTF
+   //    Initialize BOTF (bypass on the fly) signals
+   //    bypass_valid:  True (1) iff bypassed_data is valid
+   //    bypassed_rnum: Use target row number (index) of valid data as an ID.
+   //    bypassed_data: Valid data lives here instead of 'rnum' in SRAM.
+   //;   # ...
+   //    'x means "set all bits to x", or so I'm told.
    initial begin
-   //; for (my $i=0; $i<$nbanks; $i++) {
+   //;   for (my $i=0; $i<$nbanks; $i++) {
       bypass_valid[`$i`]=1'b0;  bypassed_rnum[`$i`]=`$rwidth`'b0;  bypassed_data[`$i`]='x;
-   //; }
+   //;   }
    end
    //; } else {
-   // Placeholder for initialization of optional BOTF signals, unused in this configuration.
+   //    Placeholder for initialization of optional BOTF signals, unused in this configuration.
    //; }
 
    // END WIRE DECLARATIONS____________________________________________________
@@ -419,79 +372,53 @@ module `mname`
    // Placeholder for optional BOTF signals, unused in this configuration.
    //; }
 
-
-//;# unused/old?
-//;#    //; if ($BYPASS_MECHANISM eq 'old') {
-//;#    //////////////////////////////////////////////////////////////////////////////////
-//;#    // If suppress_wz was set (at posedge clk above), we suppress write to the
-//;#    // indicated SRAM.  Contents originally destined for SRAM now go to bypass_buffer.
-//;# 
-//;#    // If I were more paranoid, maybe each case would get its own "@ (negedge clk)"?
-//;#    always @ (posedge clk_i) begin
-//;#       //;#
-//;#       //;#  if (suppress_wz[ 0]) bypass_buffer[bufnum_i[ 0]] <= SRAM000_wr_data;
-//;#       //;#  if (suppress_wz[ 1]) bypass_buffer[bufnum_i[ 1]] <= SRAM001_wr_data;
-//;#       //;#  ...
-//;#       //;#  if (suppress_wz[15]) bypass_buffer[bufnum_i[15]] <= SRAM015_wr_data;
-//;# 
-//;#       // TODO again, replace ugly repetitive $display w/a task.
-//;#       // if nunits == 1 this (suppress_wz) is one-hot (or zero)
-//;#       //; for (my $i = 0; $i < $nbanks; $i++) {
-//;#       //;     my $nn  = sprintf("%2d", $i);
-//;#       //;     my $nnn = sprintf("%03d", $i);
-//;#       //;     my $bufnum = ($nunits==1) ? "" : "[bufnum_i[$nn]]";
-//;#       //;     my $bufnum2 = ($nunits==1) ? "0" : "bufnum_i[$nn]";
-//;#       if (suppress_wz_i[`$nn`]) begin
-//;#          bypass_buffer`$bufnum` <= SRAM`$nnn`_wr_data;
-//;#          $display("fftram %6d Bypassed SRAM `$i`, wrote (bsr'%08X,bsr'%08X) to bypass_buffer[%1d]",
-//;#                $time, `ri("SRAM${nnn}_wr_data")`, `$bufnum2`);
-//;#       end
-//;#       //; }
-//;#     end
-//;#    //; } else {
-//;#    // LOOK MA NO BYPASS BUFFER mano mano
-//;#    //; }
-//;# unused/old?
-   // LOOK MA NO BYPASS BUFFER mano mano
-
    //////////////////////////////////////////////////////////////////////////////
    // wz signals.  If Anyone wants to access a given bank n (op1_bnum==1),
    // set wz(n) (to zero), unless suppress_wz says not to.
 
-   //;# The timing changes according to whether we have a 1port or a 2port SRAM.
-   //; my ($wz_event, $wz_value);
-   //;
-   //; my ($open_paren, $close_paren) = ("(", ")"); # Yeah, this is readable.
+   //;# The timing changes according to whether we have a 1port or a 2port SRAM (note dpump = 2 ports)
+
+   //; # DEFAULT values for if n_sram_ports != 1 (see below)
+
+   //; # E.g. codeblock1(2, 0) prints this:
+   //; # "         (BFLY0_op1_bnum_i == 0 ) | (BFLY0_op2_bnum_i == 0 ) |"
+   //; # "         (BFLY1_op1_bnum_i == 0 ) | (BFLY1_op2_bnum_i == 0 )"
+   //; 
+   //; sub codeblock1 {
+   //;   my ($nunits, $snum) = @_;
+   //;   my @codeblock;
+   //;   for (my $i=0; $i < $nunits; $i++) {
+   //;     @codeblock[$i] = "         (BFLY${i}_op1_bnum_i == $snum ) | (BFLY${i}_op2_bnum_i == $snum )";
+   //;   }
+   //;   print join(" |\n", @codeblock);
+   //; }
+
    //; for (my $snum=0; $snum<$nbanks; $snum++) {
    //;     my $sram = sprintf("SRAM%03d", $snum);
-   //;     if ($n_sram_ports == 2) {
-   //;         $wz_event = "clk_i";
-   //;         $wz_value = "clk_i";
-   //;     }
-   //;     elsif ($n_sram_ports == 1) {
+
+   //;     # async for 2port (inc. dpump)
+   //;     my $wz_event = "clk_i";
+   //;     my $wz_set   = "${sram}_wz <= clk_i ? 1 : 0";
+   //;     my $wz_reset = "${sram}_wz <= 1'b1";
+
+   //;     # sync for 1port
+   //;     if ($n_sram_ports == 1) {
    //;         $wz_event = "posedge clk_i";
-   //;         $wz_value = "suppress_wz_i[$snum]";
+   //;         $wz_set   = "${sram}_wz = suppress_wz_i[$snum] ? 1 : 0";
+   //;         $wz_reset = "${sram}_wz = 1'b1";
    //;     }
-   //;
-
-//; # Yet another terrible hack
-//; # Some configs work only w blocking assignments and others w non-blocking :(
-//; # $ass = "=";   # ver:  1port 2port ~dpump;  vcs  1port 2port dpump
-//; # $ass = "<=";  # ver: ~1port 2port  dpump;  vcs ~1port 2port dpump
-//; my $ass = ($SRAM_TYPE eq "TRUE_1PORT") ? "=" : "<=";
 
    always @ (`$wz_event`) begin
-      if (cycle_num == `$nbits_cycle_num`'b0) `${sram}`_wz `$ass` 1'b1; else
+      if (cycle_num == `$nbits_cycle_num`'b0) `$wz_reset`; else
       if ( fft_started & (
-   //;     my $i; for ($i=0; $i < ($nunits-1); $i++) {
-   //;     print "         (BFLY${i}_op1_bnum_i == $snum ) | (BFLY${i}_op2_bnum_i == $snum ) |\n";
-   //;     }
-   //;     print "         (BFLY${i}_op1_bnum_i == $snum ) | (BFLY${i}_op2_bnum_i == $snum )\n";
-   //;     print "      )) begin\n";
-   //;     print "         ${sram}_wz $ass $wz_value ? 1 : 0;\n";
-
+   //; #    (BFLY0_op1_bnum_i == 0 ) | (BFLY0_op2_bnum_i == 0 ) |"
+   //; #    (BFLY1_op1_bnum_i == 0 ) | (BFLY1_op2_bnum_i == 0 )"
+   //;     codeblock1($nunits, $snum);
+       )) begin
+         `$wz_set`;
+
    //;     if (($PRECOMPUTED_BYPASS == 0) && ($n_sram_ports == 1)) {    # BOTF
-          // \$display(\"FLOOF BOTF %1d okay want to write ${sram}\", \$time);
+         // \$display(\"FLOOF BOTF %1d okay want to write ${sram}\", \$time);
 
          if (bypass_time == 1) begin
             // FIXME really need a task here, no?
@@ -508,7 +435,7 @@ module `mname`
    //;     }
       end
       else begin
-         `${sram}`_wz `$ass` 1'b1;
+         `$wz_reset`;
       end
    end
    //; }

test/README.txt

@@ -0,0 +1,40 @@
+# TO RUN ALL TESTS MAYBE, DO THIS MAYBE
+source /nobackup/steveri/garnet_venv/bin/activate
+testall.sh |& tee tmp.log | less
+
+
+========================================================================
+NOTES
+
+ref: /nobackup/steveri/github/fftgen/.github/workflows/CI.yml
+
+TESTS in CI.yml: test-summary.sh?
+- cd bin/simv_analysis; make test
+- all it does is it just
+-- takes the pre-built log of an fft run
+-- runs it through a (perl!) filter to make a readable summary
+-- compares the summary to a gold model `exampes/analysis_results.txt`
+
+------------------------------------------------------------------------
+TESTS in CI.yml: test-pypl.sh --gold8
+- build and test an 8-point fft, compare to cached fftgen results
+
+TESTS in CI.yml: test-pypl.sh --comp16
+- verify perl and python models get same answer for 16-point FFT
+
+# TESTS in CI.yml: test-regress.sh -sim verilator --perl
+- exhaustive regression test, perl version
+
+# TESTS in CI.yml: test-regress.sh -sim verilator --python
+- exhaustive regression test, python version
+
+
+# OLD i think
+# ------------------------------------------------------------------------
+# TESTS in CI.yml: regressions.sh --perl?
+# - bin/golden_test.sh --perl -sim verilator > log
+# - egrep 'FAIL|ERR' test_results.log
+# 
+# TESTS in CI.yml: regressions.sh --python?
+# - bin/golden_test.sh --python -sim verilator > log
+# - egrep 'FAIL|ERR' test_results.log

test/test-pypl.sh

@@ -63,7 +63,7 @@ fftgen=$(cd $test/..; pwd)
 
 # Need numpy for python part of perl v. python tests
 # FIXME/TODO when we get rid of the python option this is no longer necessary.
-pip install numpy || echo "Could not install numpy, maybe that's okay"
+python -c 'import numpy' || pip install numpy || echo "Could not install numpy, maybe that's okay"
 
 ##########################################################################
 # --gold  =>  8-point fft with summary results

test/test-regress.sh

@@ -57,7 +57,7 @@ source $fftgen/bin/setup_genesis.sh
 
 # Need numpy for comparison with python sim maybe
 # FIXME/TODO when we get rid of the python option this is no longer necessary.
-pip install numpy || echo "Could not install numpy, maybe that's okay"
+python -c 'import numpy' || pip install numpy || echo "Could not install numpy, maybe that's okay"
 
 # See if fpu is up to date
 # TODO/FIXME this (rtl/fpu_snapshot) should really be a submodule, yes?