[dv] Move sim_tops to {tool}.hjson #4366
Conversation
sriyerg
requested review from
eunchan,
sjgitty,
tjaychen and
weicaiyang
as code owners
sriyerg
requested review from
rswarbrick,
senelson7,
rasmus-madsen,
tunghoang290780,
cindychip and
udinator
util/dvsim/utils.py
Outdated
| wildcard_re = re.compile(r"{([A-Za-z0-9\_]+)}") | ||
| wildcard_re = re.compile(r"(?<!\$){([A-Za-z0-9\_]+)}") |
There was a problem hiding this comment.
Hmm, ok, but this is a bit magical. I wonder whether it would be better to have some sort of explicit escaping (like Python's format function). So shell code that absolutely needed the braces (unusual) would end up looking like ${{foo}}.
hw/dv/tools/dvsim/dsim.hjson
Outdated
| @@ -23,6 +23,8 @@ | |||
| "-c-opts -I{DSIM_HOME}/include", | |||
| "-timescale 1ns/1ps", | |||
| "-f {sv_flist}", | |||
| // List multiple tops for the simulation. Prepend each top level with `-top`. | |||
| '''{eval_cmd} tops=(`echo {sim_tops}`); echo "${tops[@]/#/-top }"''', | |||
There was a problem hiding this comment.
While we discuss the escaping in the previous commit, you could do this with sed (and no escaping required):
echo {sim_tops} | sed -E 's/(\w+)/-top \1/g'
Maybe split out the previous commit into a separate PR, and we can figure out the best approach for that separately from this change?
hw/dv/tools/dvsim/vcs.hjson
Outdated
| // List multiple tops for the simulation. Prepend each top level with `-top`. | ||
| '''{eval_cmd} tops=(`echo {sim_tops}`); echo "${tops[@]/#/-top }"''', |
There was a problem hiding this comment.
We've got two {eval_cmd} lines in this file that both compute the same thing. Maybe put it in a separate variable to avoid the unneeded extra subprocess?
There was a problem hiding this comment.
They operate on different variables. This one prefixes -top to top level designs the the other one prefixes -dir to coverage database directories. How would merging the two work?
There was a problem hiding this comment.
Duh, I didn't read this carefully enough! You're completely right.
Most simulators accept `-top top1 -top top2` switches at compile time for indicating top level designs that need to be elaborated. Questa unfortunately needs it to be a runtime switch as opposed to build time. Support for Questa is in the works. Signed-off-by: Srikrishna Iyer <sriyer@google.com>
hw/dv/tools/dvsim/vcs.hjson
Outdated
| // List multiple tops for the simulation. Prepend each top level with `-top`. | ||
| '''{eval_cmd} tops=(`echo {sim_tops}`); echo "${tops[@]/#/-top }"''', |
There was a problem hiding this comment.
They operate on different variables. This one prefixes -top to top level designs the the other one prefixes -dir to coverage database directories. How would merging the two work?
This removes the `-top` switch with the recent changes in lowRISC#4366. Signed-off-by: Srikrishna Iyer <sriyer@google.com>
This removes the `-top` switch with the recent changes in #4366. Signed-off-by: Srikrishna Iyer <sriyer@google.com>
Fixes #4340.
This change factors
sim_topsout ofcommon_sim_cfg.hjsonin favor of being placed in{tool}.hjsoninstead. This is done to support Questa (support for which will be added soon).The first commit fixes a bug in DVSim (prevent
${..}constuct from being treated as a substitution var).The second moves
sim_topsexpansion out of the common hjson and moves it into individual tool-specific hjson. How tops are to be specified is also changed - there is no need to add-topprefix to each top level. This is factored in the tool hjson already.