VUnit / vunit

VUnit is a unit testing framework for VHDL/SystemVerilog
http://vunit.github.io/
Other
723 stars 258 forks source link

Feature Request: Add arg parse options for adding waveforms to viewer #455

Open GlenNicholls opened 5 years ago

GlenNicholls commented 5 years ago

I think it would be a nice feature to allow the user to use a flag to display all signals at a user-specified hierarchy depth when they pass the -g flag to open the GUI. I get really annoyed having to manually add waveforms when I don't yet know what I'm looking for, so I end up adding all of them. GTKWave offers groups that can be named, ModelSim offers groups as well that can be named, and Active-HDL/Riviera-Pro offer virtual signal busses (I believe with add -virtual "name_for_virtual" path/to/desired/entity/*).

Here is rough-draft of what i am referring to for GTK:

# note hierarchy depth to display in waveform viewer
set depth 2

set top_level top; # change top only if applicable

# don't change this
set num_facs  [ gtkwave::getNumFacs ]
set dump_name [ gtkwave::getDumpFileName ]
set dmt       [ gtkwave::getDumpType ]

# create list with all elements
set elements [list]
for {set i 0} {$i < $num_facs} {incr i} {
    lappend elements [ gtkwave::getFacName $i ]
}

# Now display hierarchy depth in waveform viewer
set wave [list]
foreach element $elements {
  # initial length of element
  set i_len [string length $element]

  # final length of element after removing dots
  set f_len [string length [string map {. ""} $element]]

  # thus number of dots
  set n_dots [expr {$i_len-$f_len}]

  # if the number equals the required hierarchy, then we got one
  if {$n_dots == $depth} {
    lappend wave $element
  }
}

gtkwave::/Edit/Insert_Blank
# append all to waveform viewer
set num_added [ gtkwave::addSignalsFromList $wave ]
puts "$wave"

This will go through and add all waveforms to the viewer that are at depth, so I am working on figuring out how to name groups, gtkwave::/Edit/Create_Group, from the command line for the following. It would be nice to tell VUnit that I want to see all signals at and before hierarchy depth of 3 and it would do the following in the waveform viewer:

tb_name_here { <- root depth or all tb signals
    clk
    data
    valid
}

tb_name_here.uut1 { <- UUT inside tb
    clk
    data
    valid
    nextState
}

tb_name_here.uut1.someEntity { <- some entity inside uut1
    clk
    data
    valid
    someSignalHere
    ...
}

Currently, if this feature is of interest, I can pass along all the commands to add signals to groups (except GTKWave since I haven't yet figured out how to name a group from the command line) or virtuals for ModelSim and Active-HDL since I have those already. I just haven't yet made my scripts generic enough like what I have attempted to do above for GTK.

For GTK I know I can execute my .tcl with the -g --gtkwave-args=-Sscript.tcl, but this is tedious doing this every time and also means I either need to hardcode a path to my script, or have a copy with all of my run.py scripts.

EDIT: I just figured out that if you do gtkwave::/Edit/Create_Group "some name" to name a group/comment from the command line.

eine commented 5 years ago

I think that this would be a very interesting feature. It is quite a pain to generate the modelsim.do files to automatically load the desired signals in groups into the waveform. However, I wonder how would this approach fit with the possibility to filter which signals are actually tracked (see https://ghdl.readthedocs.io/en/latest/using/Simulation.html?highlight=wave#cmdoption-ghdl-read-wave-opt). It should be somehow ensured that all the signal which are to be grouped (and shown) are not filtered out when the waveform file is generated by the simulator.

GlenNicholls commented 5 years ago

For GHDL/GTKWave specifically, the script I am working on above is simply grabbing signals from the file that has already been created, so I don't think this would be an issue since there is no call to entity names/signals specifically.

For modelsim, this is how I have approached it in the past, although I haven't yet gone through the exercise of figuring out how this would work for any name in the hierarchy path. I'm sure there is a way, I just don't know what that way is.

# 3) use specific top level
set top_level tb_ddsMixer

...

# 5) Open windows for viewing results
view wave
view signals
view structure
view objects

# 6) Display the waveforms

add wave -group Testbench $top_level/*
add wave -noupdate -expand -group Testbench -format Analog-Step -height 74 -max 8191.0 -min -8192.0 -radix signed $top_level/ddsMixerValI
add wave -noupdate -expand -group Testbench -format Analog-Step -height 74 -max 8191.0 -min 0 -radix unsigned $top_level/ddsMixerValQ
add wave -group dds $top_level/uut/*
GlenNicholls commented 5 years ago

EDIT: The following is the final script I have come up with for GTKWave. This will add all signals to a group with their path name as the name for the group. I have also added the do_add_by_name flag to allow the user to only show all signals in a specified entity name.

python run.py -g --gtkwave-args=-SaddWave.tcl
set do_add_by_name 0; # change this to 1 if adding by name. IF 1, max_depth ignored
set max_depth      5; # must be >1

# add desired entity names below
# NOTE: for entities generated by for/if, precede entity name with generate statement along
#       with the index like so: entity_gen[0] where entity_gen is what the generate statement name is.

set top tb_txbertsimple; # put your tb entity name here

lappend add_waves $top
lappend add_waves $top.allOnes
lappend add_waves $top.allZeros

# don't change below this line
set num_facs  [ gtkwave::getNumFacs ]

# create dict key/val
set dictGroup [dict create]

# search entire waveform list to find matches to add to groups
for {set i 0} {$i < $num_facs} {incr i} {
  set path [ gtkwave::getFacName $i ]

  # split path and determine how many elements there are
  set elements     [ split $path . ]
  set num_elements [ llength $elements ]

  # remove first element because redundant and last because this is the signal name, then join for matching
  set group_elements [ lreplace [ lreplace $elements 0 0 ] end end ]
  set group_name     [ join $group_elements "." ]

  # init add_wave flag so we only add match
  set add_wave 0

  # if we are adding by name, else add by hierarchy
  if {[string is true $do_add_by_name]} {

    # search add_waves to see if we have group name match that is case insensitive
    set is_string_match [lsearch -exact [ string tolower $add_waves ] $group_name]

    if {$is_string_match >= 0} {
      incr add_wave
    }

  } else {

    # if we have correct hierarchy depth, add element
    if {$num_elements <= $max_depth} {
      incr add_wave 
    }

  }

  # if we found match, append path to group key
  if {$add_wave > 0} {
    # create dict with key:<group_name> and value:<path>
    set key   $group_name
    set value $path

    # append element to group key
    dict lappend dictGroup $key $value
  }
}

set first 0

foreach {k v} $dictGroup {
  set num_added [ gtkwave::addSignalsFromList $v ]

  # GTK doesn't highlight the first set of signals added so nothing gets grouped
  if {$first == 0} {
    gtkwave::/Edit/Highlight_All
  }
  gtkwave::/Edit/Create_Group "$k"
  gtkwave::/Edit/Toggle_Group_Open|Close
  gtkwave::/Edit/UnHighlight_All

  incr first
}
tmeissner commented 5 years ago

I use a tcl file for GTKWave also, but I add the signals manually.

Nice idea to put some automation to this. But I always would use this with max_depth set to 1. Simply because I don't want all signals in my waveform. I never had a use case to add all of them, even not at work with Modelsim/Questa. For many designs with a lot of signals, I would get a lot of signals in the waveform viewer that I don't need - most of them would even deviate me from the important ones.

GlenNicholls commented 5 years ago

@tmeissner I see your point, however, I somewhat disagree with that notion. Before I get to the solution to your problem, let me preface this with my argument against that idea for ModelSim/Questa/Aldec. In these simulators, when you are bug hunting and forget to add a waveform, you have to restart the simulation waveform after adding a new signal which I find to be a huge time sink when you know a bug isn't cropping up until 2ms into simulation. I've found this to be exponentially worse when trying to debug huge designs (or with things like PCIe/10/20/40G ethernet etc.) where you absolutely have to include a lot of your design because unit testing didn't catch something you're having issues tracing. If I know something is broken inside a specific component, but I don't know exactly where, it is nice having all the waveforms present to sift through.

I do see your point, but I find this type of thing tremendously useful when I'm jumping into a legacy simulation that takes literally ages to run :/ I have added a flag for do_add_by_name which can be set to 1 and you can enter the path to all the entities you want to display. I have also modified it to toggle the groups closed so that once the simulator comes up it is nice and clean, so all you have to do is select a group and press T to toggle it open. Give it a shot and let me know if you find this useful moving forward.

tmeissner commented 5 years ago

I also understand your point. I will give a try. I assume, that the script doesn’t rely on vunit? Because I don’t have a running vunit project at home. I would try it with a plain vhdl testbench of a project of my own.

GlenNicholls commented 5 years ago

You are correct, this does not rely on VUnit. This is just passing commands onto GTKWave to basically perform the following:

The only limitation is my script expects the signal path name in each facility to be deliminated by a ".". I do not know how this would affect a .vcd wave format vs. the VUnit default .ghw. However, this I believe can be as simple as changing the delimiter to a "/" if that is what .vcd uses.

tmeissner commented 5 years ago

@GlenNicholls I've tested the tcl script with a test design of mine. It works, however, every vector I get numerous times in the wave window. data_i[0:127] for example 128 times. This is the case with every vector I have in the design. I assume that's not what we want - I definitely not ;) I will have a look in the tcl file to fix that, but I'm not an expert in tcl ;)

I've attached a screenshot which shows the behaviour:

gtkwave

tmeissner commented 5 years ago

@GlenNicholls I've made a (very ugly & hackish) change to your script that vectors are only added once instead of multiple times. Maybe (definitely) there is a better way to do that, possibly GTKwave has an API function which can be used to find out of a signal is a vector or something like that. The change looks like this. Instead of adding all paths:

# if we found match, append path to group key
  if {$add_wave > 0} {
    # create dict with key:<group_name> and value:<path>
    set key   $group_name
    set value $path

    # append element to group key
    dict lappend dictGroup $key $value
  }

I filter out the equal ones by cutting the last vector notation ([]) and comparing the resulting path to the last processed one:

 # if we found match, append path to group key
  if {$add_wave > 0} {

    set elements [ split $path \[ ]

    if { [ llength $elements ] > 1 } {
        set var [ lreplace $elements end end ]
        set path [ join $var \[ ]
    }

    if {$old_path ne $path} {
        # create dict with key:<group_name> and value:<path>
        set key   $group_name
        set value $path
        set old_path $path

        # append element to group key
        dict lappend dictGroup $key $value
    }
  }

I know that it's ugly and I assume that that don't works for every case. But for my design it works as desired. I only get the signals I want in my wave viewer:

gtkwave_1

GlenNicholls commented 5 years ago

Interesting, I did not realize GTK would add a duplicate, I will revisit this soon. I think the API has a setting to remove all duplicate waveforms, but I'm not positive. Either way, this looks like a reasonable fix to me. There could probably be fancier checking on the signal names as this might break when adding signals to an entity that was generated with a for generate statement. I am also not an expert with tcl so there is probably a way nicer way of accomplishing my objectives. I just know how to make it do what I want and that's about it

GlenNicholls commented 5 years ago

I have an update for ModelSim that I will be testing and modifying to somewhat replicate the functionality above. I found this which is basically what I did for GTK.

GlenNicholls commented 5 years ago

If you're curious, I have created a script for ModelSim. This one does not support adding by entity/path names yet as I have a few ideas for keeping this script in a single location. My plan is to eventually pass these procedures information from each test to add everything to the waveform viewer, but there is some functionality I would like to see in VUnit before I work on that.

set max_depth 3; # must be >0

# create dict key/val
set group_names [ list ]

# get all signals
set all_signals [ find signals * -r ]

# define procedure to create unique list
proc list_unique {list} {
  array set included_arr [list]
  set unique_list [list]
  foreach item $list {
    if { ![info exists included_arr($item)] } {
      set included_arr($item) ""
      lappend unique_list $item
    }
  }
  unset included_arr
  return $unique_list
}

# parse list to grab groups we want to add to waveform viewer
foreach path $all_signals {
  # split path and determine how many elements there are
  set elements     [ split $path / ]

  # remove first element because redundant and last because this is the signal name, then join for matching
  set group_elements [ lreplace $elements end end ]
  set group_name     [ join $group_elements "/" ]

  # determine number of elements
  set num_elements [ llength $group_elements ]
  incr num_elements -1

  if {$num_elements <= $max_depth} {
    lappend group_names $group_name
  }
}

# organize list and remove duplicates
set sorted_group [ lsort -increasing [ list_unique $group_names ] ]

foreach { group_name } $sorted_group {
  set CMD "add wave -noupdate -group $group_name $group_name/*"
  echo $CMD
  eval $CMD
}
umarcor commented 5 years ago

As commented in #459, I tried the scripts in this issue.

The version in https://github.com/VUnit/vunit/issues/455#issuecomment-470797811 does replicate all the std_logic_vector signals as many times as the number of bits. Unfortunately, the fix by @tmeissner, complained about can't read "old_path": no such variable. I added set old_path "" to fix it. It does not complain now, but it does neither work as expected: vectors are still replicated. It works now.

Apart from that, it seems that a single level of hierarchy is created. I think it'd be useful to make this 'flat' visualization optional, so it's also possible to show the hierarchy in the waveform.

@GlenNicholls, do you have these scripts (for GtkWave, ModelSim/QuestaSim, etc.) available at any public repo?

cclienti commented 5 years ago

Hello,

I hope the following will help you.

The same grouping problem occurs to me with gtkwave and I solved in my python module (wavedisp) to generate wave file descriptions for gtkwave/modelsim/rivierapro

https://github.com/cclienti/wavedisp (Example of use: https://www.wavecruncher.net/wavedisp)

The major issue that I figured out to group signal was the missing hierarchy of names after added them in gtkwave. But there is a very simple solution that does not require any development.

You can add at the beginning of the tcl script loaded by gtkwave a line that shows the complete hierarchy in the added signal panel. After that, the grouping command will require the full hierarchy name in order to group signals.

gtkwave::/Edit/Set_Trace_Max_Hier 0

At the you can restore the value

gtkwave::/Edit/Set_Trace_Max_Hier 1

A small example from one of my design that you can invoke with the '-T' option of gtkwave:

# Wavedisp generated gtkwave file
gtkwave::/Edit/Set_Trace_Max_Hier 0

gtkwave::addSignalsFromList [list {dclkfifolut_tb.DUT.read_ptr_gray.bin}]
gtkwave::addSignalsFromList [list {dclkfifolut_tb.DUT.read_ptr_gray.gray}]
gtkwave::/Edit/UnHighlight_All
gtkwave::/Edit/Highlight_Regexp {^dclkfifolut_tb.DUT.read_ptr_gray.bin}
gtkwave::/Edit/Highlight_Regexp {^dclkfifolut_tb.DUT.read_ptr_gray.gray}
gtkwave::/Edit/Create_Group {read_ptr_gray}
gtkwave::/Edit/UnHighlight_All

gtkwave::/Edit/Set_Trace_Max_Hier 1
GlenNicholls commented 4 years ago

@umarcor reference the file I attach to #615

GlenNicholls commented 4 years ago

@cclienti

You can add at the beginning of the tcl script loaded by gtkwave a line that shows the complete hierarchy in the added signal panel. After that, the grouping command will require the full hierarchy name in order to group signals.

So I am working on polishing the GTK interface and am running into problems with records. Do you know any methods to recognize when a signal is a record? My assumption for creating a group name is it is the common path before the final value

tb_unit.clk
tb_unit.control.clk
tb_unit.control.rdptr

Here, My script recognizes that tb_unit.control is common between clk and rdptr. However, control is simply a record inside tb_unit, so I've been stuck trying to figure out how to recognize records.

Ideally, I would see the following groups I create:

tb_unit
--clk
--control
----clk
----rdptr

But instead, I see

tb_unit
--clk
tb_unit.control
--clk
--rdptr
GlenNicholls commented 4 years ago

This is in relation to https://github.com/VUnit/vunit/pull/665 and also #455

After debugging an IP core from a vendor, I found that with cores that have massive hierarchy trees (With tens or hundreds of files), it is easier to navigate the GUI when adding signals to the waveform viewer replicates the design hierarchy. The PR noted above currently puts the path of the entity in the group name, but this became difficult to navigate. Instead, it would be better if nested groups were created so it would be easier to navigate and cross-reference signals:

image

For each entity in the hierarchy, it is much cleaner if it is added as a sub-group to the parent group. I began the notation of adding all signals to the signal group in ModelSim for the parent entity.

In short, when adding all waves by depth, the wave viewer should mock the hierarchy list. This will be quite a bit of work and debugging I think since there will need to be lists/dictionaries to keep track of the entire hierarchy. In ModelSim and Aldec tools, this is easier since it is possible to add waves with the * wildcard. Not only this, but they have TCL commands for getting all entities which also contain the entire path. GTKWave on the other hand is much more manual as the script would have to create its own understanding of the design hierarchy more than it already does. Currently, dictionaries are created with library:[<list of signals>], but because all signals are a single bit, we not only have to continue dealing with re-constructing the vector/record/array, but also keep track of how to properly nest groups.