#918 Problems with NetPartSelect:PV concatenation optimisations

devel
closed-fixed
nobody
None
5
2013-02-03
2013-01-25
Adrian Wise
No

The git master version of icarus fails to simulate correctly a predominantly gate-level simulation of a historic mini-computer that simulates correctly using 0.9.6.

I happen to be at this revision:

  • 922b74b Fix uninitialized buffer when drawing a real VPI argument

The circuit uses a technology (DTL) that relies on strength resolution, it is common for multiple gates to drive the same node - logic 1 being just a resistive pull-up so the node will be high unless one or more gates is diving it low.

The problem seems to be associated with the test-bench and a wrapper around the net-listed schematic where several series of individual schematic signals are gathered together into buses - though I may be mistaken in drawing this conclusion.

Deploying git bisect reveals that the problem was first introduced by:

  • 098bbee Support collapse of PartSelect::PV to concatenation

However, if I patch elab_net.cc at the HEAD of the master branch to prevent that optimisation occurring, the bug is apparently still there.

Using bisect again, I found a second, more recent, change that causes the (same?) problem:

  • 367d7bf Blend NetPartSelect(PV) objects into NetConcat

The symptoms in each case appear to be the same - the simulation degenerates into X's at around time 6602.5ps. The first nodes to go being the non-zero bits of the M register, M[01:16]FF_p (see the waveforms produced by the test case).

If I patch the HEAD of the master branch to disable both of these optimisations like this:

diff --git a/cprop.cc b/cprop.cc
index a37e669..cf09181 100644
--- a/cprop.cc
+++ b/cprop.cc
@@ -211,7 +211,7 @@ static bool compare_base(NetPartSelect*a, NetPartSelect*b)
  */
 void cprop_functor::lpm_part_select(Design*des, NetPartSelect*obj)
 {
-      if (obj->dir() != NetPartSelect::PV)
+  //if (obj->dir() != NetPartSelect::PV)
            return;

       NetScope*scope = obj->scope();
diff --git a/elab_net.cc b/elab_net.cc
index 6d39f8f..a2ff09c 100644
--- a/elab_net.cc
+++ b/elab_net.cc
@@ -741,7 +741,7 @@ NetNet* PEIdent::elaborate_lnet_common_(Design*des, NetScope*scope,
                  des->add_node(sub);
                  sub->set_line(*this);
                  connect(sub->pin(0), subsig->pin(0));
-                 collapse_partselect_pv_to_concat(des, sig);
+                  //collapse_partselect_pv_to_concat(des, sig);
            }
            sig = subsig;
       }

then the simulation runs as expected (there's activity on most of the buses throughout the entire duration of the simulation).

I'm afraid that I've been unable to isolate a small testcase that illustrates the problem. The testcase attached has the following files:

ccc.v - Gate library
h316_cpu_bus.v - Wrapper around the gate-level netlist
h316_cpu.v - The gate-level netlist
h316.f - Config file
h316_tb.v - Testbench
h316_waves.sav - Waveform definitions for gtkwave
Makefile - makefile
patch_ab16cct4.vmem - Data (a test program) read into testbench

Assuming an icarus and gtkwave on the path, simply untar and type 'make' to run the example.

If it is helpful to understand the circuit better, the schematics are available here:

http://www.series16.adrianwise.co.uk/hardware/pdf/h316_all_a4.pdf

1 Attachments

Discussion

  • Martin Whitaker

    Martin Whitaker - 2013-01-27

    Here is a simple test case that exposes the problem introduced by

    • 098bbee Support collapse of PartSelect::PV to concatenation
     
  • Martin Whitaker

    Martin Whitaker - 2013-01-27

    This was an attempt to expose the other problem, but instead exposes a case where the compiler generates invalid code.

     
  • Martin Whitaker

    Martin Whitaker - 2013-01-29

    Here is a simple test case that exposes the problem introduced by

    • 367d7bf Blend NetPartSelect(PV) objects into NetConcat
     
  • Cary R.

    Cary R. - 2013-01-29
    • priority: 5 --> 7
     
  • Cary R.

    Cary R. - 2013-01-29

    Since there does not appear to be any workarounds and this is producing incorrect results without an error/warning I'm going to increase the priority.

     
  • Stephen Williams

    I've pushed to git master a fix for most of the issues here. I've fixed br918a and br918c (and added them to the test suite.) I haven't fixed br918b yet, but this is no longer a priority-7 issue so I'll turn down the heat a bit.

     
  • Stephen Williams

    • priority: 7 --> 5
     
  • Stephen Williams

    br918b is fixed in git master, and br918b.v is added to the ivtest suite.

     
  • Stephen Williams

    • status: open --> closed-fixed
     

Log in to post a comment.