Improved HDL simulation flow for Questa #47

nielshaandbaek · 2021-12-06T08:28:39Z

Overview

The purpose of the following PR is to provide an improved HDL simulation flow. The main changes are:

Adds support for dbt run and dbt test in addition to dbt build
Now, dbt run is used for starting the simulation in UI mode
And dbt test is used for running unit tests (i.e. batch mode)
New flags are available including the ability to define the parameters of the top-level design from the Simulation target.
Coverage information can also be activated.

How to use

Run the usual dbt build command to see a list of available build targets. Each target now has additional information as shown below:

$ dbt build
...
//exp-compute/vta/hw/dta_v1/sim/dta_core/DtaCoreSimulation  //exp-compute/vta/hw/dta_v1/sim/dta_core/DtaCoreSimulation ( Name: DtaCore TestCases: add-10x10-16ch.json add-2x2-16ch.json conv-10x10-16ch.json conv-1x1-16ch.json conv-3x3-16ch.json conv-3x3-1ch.json conv-big.json maxpool-10x10-16ch.json maxpool-3x3-16ch.json relu-2x2-16ch.json resize-10x10-16ch.json resize-2x2-16ch.json )
...
//exp-compute/vta/hw/dta_v1/sim/common/axi/axi_register_bank/Simulation  //exp-compute/vta/hw/dta_v1/sim/common/axi/axi_register_bank/Simulation ( Name: AxiRegisterBankSimulation Params: default AxiDataWidth64 AxiAddrWidth32 DefaultMask RegisterCount1 RegisterCount16 )
...

The additional information will be discussed in more detail below.

Test

Use the dbt test command to execute unit tests. The command will execute the simulator in batch mode and create code coverage if selected.

Run

Use the dbt run command to execute tests in GUI mode. The simulator will open the UI and read in an optional waveform configuration file. Note that it will not automatically store code coverage data at the end of the simulation.

Coverage

Code coverage may be generated using the dbt test with the flag questa-coverage=true. Code coverage will produce an HTML report of the entire design in the BUILD/OUTPUT-XXX folder as well as a coverage database in UCDB format. The coverage database can be viewed using the vsim -viewcov <database> command.

Note that the tool will create a single coverage database for every simulation target, which merges the results of all simulation runs from e.g. multiple testcases and/or multiple parameter sets.

Params

The Params enables different named parameter sets to be defined. The option is useful for modules with parameters where a single top-level testbench may be used to verify different configurations of the modules. To add a parameter set use the Params field of the hdl.Simulation target like this:

var Simulation = hdl.Simulation{
	Name:   "AxiRegisterBankSimulation",
	Srcs:   ins("tb.sv"),
	Ips:    Ips,
	Top:    "tb",
	Params: hdl.ParamMap{
		"default": {}, 
		"AxiDataWidth64": {"AxiDataWidth": "64"},
		"AxiAddrWidth32": {"AxiAddrWidth": "32"},
		"DefaultMask":    {
			"DefaultMask": "1",
			"DefaultReadOnly": "1",
		},
		"RegisterCount1":  {"RegisterCount": "1"},
		"RegisterCount16": {"RegisterCount": "16"},
	},
	WaveformInit: in("wave.do"),
}

The Params field is a map with strings as keys. The key is the name of the parameter set. The content of each entry should themselves be a map from strings to strings. The key is the parameter name and the value is the parameter value. The parameters of a parameter set will be applied during the vopt step using the -G option, e.g. -G AxiDataWidth=64.

Parameters can be selected using the -params flag. For example, to select RegisterCount16 as the parameter set one should execute e.g.

$ dbt test compute/vta/hw/dta_v1/sim/common/axi/axi_register_bank/Simulation : -params=RegisterCount16

Note that all parameter sets are always compiled, so make sure that they are syntactically correct. You can select multiple parameter sets to execute by specifying them as a comma separated list.

TestCases

Some testbenches require the use of an external test generator script for creating input data and expected output. The generator can be entered using the TestCaseGenerator and TestCasesDir fields of the hdl.Simulator target, like this:

var DtaCoreSimulation = hdl.Simulation{
	Name: "DtaCore",
	Srcs: ins(
		"tb.sv",
	),
	Ips: []hdl.Ip{
		util.Logger,
		interfaces.Stream,
		common.DtaPackage,
		rtl.Dta,
		fetch.FetchTest,
		issue.IssueTest,
		dma.DmaTest,
		execute.ExecuteTest,
		axi_register_bank.AxiRegBankTestPackage,
	},
	TestCaseGenerator: in("../../../../../../../DEPS/exp-dta-compiler/testgen"),
	TestCasesDir:      sim.TestCases,
}

The TestCasesDir is optional. However, if defined, every file in the directory will be listed as an optional testcase. By default, if TestCaseGenerator is defined, the generator will always be executed. If no testcase is specifically selected on the command line and TestCasesDir is defined, then the first file in the directory will be used as the default testcase.

To execute a specific testcase do

$ dbt test //exp-compute/vta/hw/dta_v1/sim/dta_core/DtaCoreSimulation : -testcases=maxpool-10x10-16ch.json

Multiple testcases can be specified using a comma separated list, e.g. `-testcases=default.json,maxpool-10x10-16ch.json'.

ganoam

Has an informal first look at this proposal.
Thanks Niels! I like this a lot. This is much more flexible than our current solution. Please go ahead and let me know when this is ready to go.

RULES/hdl/questa.go

nielshaandbaek · 2021-12-07T14:21:11Z

I will test it on Jenkins tomorrow and then we can merge it.

ganoam

This is amazing, thanks!

I've got no useful comments on the code, it looks good to me. Maybe we can have @jf have a look from the software perspective.

I suggest that we test this for a day or two without merging, to see if we can think of anything else we'd like to add.

RULES/hdl/questa.go

ganoam · 2021-12-08T08:09:59Z

RULES/hdl/questa.go

+			                                        " -directive -cvg -codeAll -testname %s" + 
+																							" -instance %s %s.ucdb\"", 
+																							testcase, rule.Instance(), coverage_db))


Nit: Weird formatting

RULES/hdl/questa.go

ganoam · 2021-12-08T08:30:43Z

If no testcase is specifically selected on the command line and TestCasesDir is defined, then the first file in the directory will be used as the default testcase.

Is there a way to execute all testcases? And similarly, is there a way to execute whatever test-case (or all) using all parameter sets?

jf · 2021-12-08T12:46:00Z

This is amazing, thanks!

I've got no useful comments on the code, it looks good to me. Maybe we can have @jf have a look from the software perspective.

has somebody been claiming "jf" here? The real "jf" is this guy, I'm sorry.

nielshaandbaek · 2021-12-08T12:46:40Z

The newest version now has the capability of simulating all parameter sets and testcases. The behavior is that if no testcase or parameter set is specified, then all will be simulated.

It now also outputs OK when the simulation succeeds for a little visual feedback.

ganoam · 2021-12-08T13:01:03Z

Haha, sorry @jf. I meant @jfrohnhofen. :)

Thanks, Niels. That's perfect.

jfrohnhofen

Only looked at the code from a DBT perspective, not from the perspective of a potential user.

Two overall remoarks: The formatting seems to be off in a couple of places.
Some functions in questa.go are only used inside his package and could be made private by giving them lowercase names.

jfrohnhofen · 2021-12-08T13:19:13Z

RULES/hdl/utils.go

-	return strings.HasSuffix(path, ".v") || strings.HasSuffix(path, ".sv") || strings.HasSuffix(path, ".vhd")
+	return strings.HasSuffix(path, ".v") ||
+	       strings.HasSuffix(path, ".sv") ||
+				 strings.HasSuffix(path, ".vhd")


The formatting looks weird

jfrohnhofen · 2021-12-08T13:19:24Z

RULES/hdl/utils.go

+
+func IsVhdl(path string) bool {
+	return strings.HasSuffix(path, ".vhdl") ||
+				 strings.HasSuffix(path, ".vhd")


jfrohnhofen · 2021-12-08T15:52:21Z

RULES/hdl/questa.go

+  cmd_preamble := ""
+
+  // Default log file
+	log_file_suffix := "vsim.log"


jfrohnhofen · 2021-12-08T15:54:46Z

RULES/hdl/questa.go

+  // Parse additional arguments
+  for _, arg := range args {
+    if strings.HasPrefix(arg, "-testcase=")  && rule.TestCaseGenerator != nil {
+			var testcase string


nielshaandbaek · 2021-12-08T17:36:20Z

I hope to have fixed the formatting and made the questa.go functions private.

ganoam

Ok, I finally took this PR for a spin. I like the new capabilities, and the new UI very much, thanks a million for taking the initiative.
However, I have discovered some problems with the code so far.

It seems the parsing of the command line flags does not yet work propperly. More precisely, I've made the following observations:

The argument for the verbosity flag is ignored. The simulation runs in silent mode no matter what.
Arguments outside the allowed range {"none", "low", "medim", "high") are also ignored - i.e. they don't make the command crash, as thy should. The same goes for the -params argument.
Arguments to the -params flag are "somewhat ignored": it seems that a simulation for every parameter set is triggered every second time the command is issued. On the alternate invokation, a specific parameter set is simulated, but not always the rght one.

I'm appending a snipped from a couple of consecutive trials I did. I'll not post my full setup publicly, please ping me privately for that.

ddln-ng[code/exp-compute] $ ninja: build stopped: interrupted by user.
dbt test //exp-compute/vta/hw/dta_v1/sim/dma/DmaSimulation -v -seed=4199734666 -verbosity=hih -params=SramLatency1
[2/3] Created exp-compute/vta/hw/dta_v1/sim/dma/DmaSimulation:
BUILD/OUTPUT-069C378E/DMA/SramLatency10_vopt.log
BUILD/OUTPUT-069C378E/DMA/SramLatency1_vopt.log
BUILD/OUTPUT-069C378E/DMA/default_vopt.log
BUILD/OUTPUT-069C378E/DMA/exp-compute/vta/hw/dta_v1/sim/dma/tb.sv.log
[2/3] Testing exp-compute/vta/hw/dta_v1/sim/dma/DmaSimulation:
Testcase SramLatency10:^C                         
ddln-ng[code/exp-compute] $ ninja: build stopped: interrupted by user.
dbt test //exp-compute/vta/hw/dta_v1/sim/dma/DmaSimulation -v -seed=4199734666 -verbosity=hih -params=SramLatency1
[2/3] Created exp-compute/vta/hw/dta_v1/sim/dma/DmaSimulation:
BUILD/OUTPUT-069C378E/DMA/SramLatency10_vopt.log
BUILD/OUTPUT-069C378E/DMA/SramLatency1_vopt.log
BUILD/OUTPUT-069C378E/DMA/default_vopt.log
BUILD/OUTPUT-069C378E/DMA/exp-compute/vta/hw/dta_v1/sim/dma/tb.sv.log
[2/3] Testing exp-compute/vta/hw/dta_v1/sim/dma/DmaSimulation:
Testcase default:^C
ddln-ng[code/exp-compute] $ dbt test //exp-compute/vta/hw/dta_v1/sim/dma/DmaSimulation -v -seed=4199734666 -verbosity=high -params=SramLatency1
[2/3] Created exp-compute/vta/hw/dta_v1/sim/dma/DmaSimulation:
BUILD/OUTPUT-069C378E/DMA/SramLatency10_vopt.log
BUILD/OUTPUT-069C378E/DMA/SramLatency1_vopt.log
BUILD/OUTPUT-069C378E/DMA/default_vopt.log
BUILD/OUTPUT-069C378E/DMA/exp-compute/vta/hw/dta_v1/sim/dma/tb.sv.log
[2/3] Testing exp-compute/vta/hw/dta_v1/sim/dma/DmaSimulation:
Testcase default:^C
ddln-ng[code/exp-compute] $ ninja: build stopped: interrupted by user.

ganoam · 2021-12-13T11:16:27Z

RULES/hdl/questa.go

+    if rule.WaveformInit != nil {
+      do_flags = append(do_flags, rule.WaveformInit.String())
+    }


While we're at it: The WaveformInit rule parameter is somewhat superfluous: Can we work on the convention that waveformInit scripts are always called sth, (e.g. waves.do)? Then we could make do without that parameter. Just probe the source dir and if it exists, add the flag.

wrt. to your tests, keep in mind that runtime parameters should come after a ":" in the command line. For example:

❯ dbt test //exp-compute/vta/hw/dta_v1/sim/fetch/Simulation : -verbosity=foo 2022/01/10 10:49:48 invalid verbosity flag 'foo', only 'low', 'medium', 'high' or 'none' allowed! exit status 1 Error: Failed to run generator: exit status 1. A fatal error occured. Exiting...

The problem with not having a WaveformInit rule is that probing the source directory from dbt is a bit difficult. I will need to see what I can do.

For your example, please use a simulation target like this:

var Simulation = hdl.Simulation{ Name: "dma", Srcs: ins( "tb.sv", ), Ips: []hdl.Ip{ util.Logger, dma.Dma, interfaces.Stream, DmaTest, }, Params: hdl.ParamMap{ "default": {}, "SramLatency1": {"SramLatency": "1"}, "SramLatency10": {"SramLatency": "10"}, }, WaveformInit: in("waves.do"), }

with a command line like this:

dbt test //exp-compute/vta/hw/dta_v1/sim/dma/Simulation : -seed=4199734666 -params=SramLatency1

you can then add -verbosity=high too if you like.

and make sure you do a dbt sync --update too!

ganoam

I have no relevant comments on the code. It looks good to me. I'll take this for a spin now. Will give more feedback later if I encounter any problems in practise, or approve if not.

RULES/hdl/questa.go

ganoam · 2021-12-22T14:25:47Z

Thanks a lot for your work. After about half a day of testing, I've got a few more comments. I've encountered three issues:

Invoking dbt test on a target runs the simulate command regardless weather or not the build step has succeeded. It doesn't matter in the big picture; the vsim command will just fail. But I believe it would be nicer to abort, if possible.
The arguments passed to dbt test and dbt run are not properly parsed. I tested the -verbosity and testcase arguments on a top-level test. They had no effect.
Again regarding top-level tests: The testgen binary is not taken into account in dependency management. If it gets out of date, it should be recompiled automatically if possible.

ganoam · 2022-01-10T11:14:53Z

Thanks for the clarifications. Works now like a charm.

ganoam · 2022-01-10T16:45:05Z

RULES/hdl/xsim.go

+	cmd := fmt.Sprintf("{ echo -n %s && xsim %s %s %s && "+
+		"{ { ! grep -q FAILURE %s; } && echo PASS; } }",
+		cmd_echo, vsim_flags, rule.Name+params, cmd_devnull, log_file.String())


I believe this only works reliably with our new testing environment. But that's not yet implemented consistently.

RULES/hdl/questa.go

…agement instead of scripts.

… updated to merge coverage.

…overage database.

…gets are evaluated.

…s fail. Also made naming of directories a bit more sane.

…and params. Improved dbt build output to reflect command usage. Outputs coverage database location when enabled.

nielshaandbaek assigned ganoam Dec 6, 2021

ganoam reviewed Dec 6, 2021

View reviewed changes

RULES/hdl/questa.go Show resolved Hide resolved

RULES/hdl/questa.go Outdated Show resolved Hide resolved

ganoam reviewed Dec 6, 2021

View reviewed changes

RULES/hdl/questa.go Show resolved Hide resolved

nielshaandbaek force-pushed the nh-improved-hdl-flow branch from 0686160 to 33550a7 Compare December 7, 2021 10:07

nielshaandbaek requested a review from juanjerez December 7, 2021 14:21

ganoam reviewed Dec 8, 2021

View reviewed changes

ganoam approved these changes Dec 8, 2021

View reviewed changes

jfrohnhofen reviewed Dec 8, 2021

View reviewed changes

ganoam mentioned this pull request Dec 9, 2021

Add options for preprocessor macros, module parameters and custom compile-time flags #27

Closed

ganoam suggested changes Dec 17, 2021

View reviewed changes

nielshaandbaek force-pushed the nh-improved-hdl-flow branch from 69a9002 to b4b02f0 Compare December 21, 2021 15:58

ganoam reviewed Dec 22, 2021

View reviewed changes

RULES/hdl/questa.go Outdated Show resolved Hide resolved

RULES/hdl/questa.go Outdated Show resolved Hide resolved

ganoam reviewed Dec 22, 2021

View reviewed changes

RULES/hdl/questa.go Show resolved Hide resolved

nielshaandbaek force-pushed the nh-improved-hdl-flow branch from b4fe0da to da6abfc Compare January 10, 2022 10:20

ganoam approved these changes Jan 10, 2022

View reviewed changes

ganoam reviewed Jan 10, 2022

View reviewed changes

nielshaandbaek force-pushed the nh-improved-hdl-flow branch 2 times, most recently from b4e9fe8 to 9f708cf Compare January 14, 2022 19:43

Niels added 4 commits January 14, 2022 20:46

Updated to use vlog, vcom, vopt and vsim directly with dependency man…

db4e96d

…agement instead of scripts.

Updated to support testcases.

78531e5

Updated to make the testcase generator and test cases work.

dd2e494

Updated to compile all parameter sets enabling run-time selection and…

39e4ed2

… updated to merge coverage.

Niels added 14 commits January 14, 2022 20:46

Add VsimFlags option.

cb7073e

Fixed whitespace. Updated to be able to simulate all parameter sets.

2c6ffb1

Fixing formatting using 'go fmt' and converted tabs to spaces.

55abec7

Made functions not needed elsewhere private.

a4a960f

Updated how coverage data is generated; added flag to generate HTML c…

6a9a53f

…overage database.

Added Vivado xsim support.

269f40e

Updated formatting.

6942c56

Added descriptions to variables.

cfca5d2

Removed check for valid parameter set as it doesn't work when all tar…

1ca5865

…gets are evaluated.

Changed name of optimized design to avoid name collisions.

3d1177f

Updated to run final blocks too.

8b41d80

Updated to also add a dependency on the testcase generator if available.

c219f56

Fixed problem where a compilation error did not make the build proces…

5f4f4e5

…s fail. Also made naming of directories a bit more sane.

Updated based on review feedback to accept comma-separated testcases …

ad3a076

…and params. Improved dbt build output to reflect command usage. Outputs coverage database location when enabled.

nielshaandbaek force-pushed the nh-improved-hdl-flow branch from 9f708cf to ad3a076 Compare January 14, 2022 19:46

nielshaandbaek merged commit 6e29332 into master Jan 14, 2022

nielshaandbaek deleted the nh-improved-hdl-flow branch January 17, 2022 16:07

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Improved HDL simulation flow for Questa #47

Improved HDL simulation flow for Questa #47

nielshaandbaek commented Dec 6, 2021 •

edited

Loading

ganoam left a comment

nielshaandbaek commented Dec 7, 2021

ganoam left a comment

ganoam Dec 8, 2021

nielshaandbaek Dec 8, 2021

ganoam commented Dec 8, 2021

jf commented Dec 8, 2021

nielshaandbaek commented Dec 8, 2021

ganoam commented Dec 8, 2021

jfrohnhofen left a comment

jfrohnhofen Dec 8, 2021

jfrohnhofen Dec 8, 2021

jfrohnhofen Dec 8, 2021

jfrohnhofen Dec 8, 2021

nielshaandbaek commented Dec 8, 2021

ganoam left a comment

ganoam Dec 13, 2021

nielshaandbaek Jan 10, 2022

nielshaandbaek Jan 10, 2022

nielshaandbaek Jan 10, 2022

nielshaandbaek Jan 10, 2022

ganoam left a comment

ganoam commented Dec 22, 2021 •

edited

Loading

ganoam commented Jan 10, 2022

ganoam Jan 10, 2022

Improved HDL simulation flow for Questa #47

Improved HDL simulation flow for Questa #47

Conversation

nielshaandbaek commented Dec 6, 2021 • edited Loading

Overview

How to use

Test

Run

Coverage

Params

TestCases

ganoam left a comment

Choose a reason for hiding this comment

nielshaandbaek commented Dec 7, 2021

ganoam left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ganoam commented Dec 8, 2021

jf commented Dec 8, 2021

nielshaandbaek commented Dec 8, 2021

ganoam commented Dec 8, 2021

jfrohnhofen left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nielshaandbaek commented Dec 8, 2021

ganoam left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ganoam left a comment

Choose a reason for hiding this comment

ganoam commented Dec 22, 2021 • edited Loading

ganoam commented Jan 10, 2022

Choose a reason for hiding this comment

nielshaandbaek commented Dec 6, 2021 •

edited

Loading

ganoam commented Dec 22, 2021 •

edited

Loading