-
Notifications
You must be signed in to change notification settings - Fork 923
[dvsim,hjson] Remove all dead code 'design_level' references/overrides #27486
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[dvsim,hjson] Remove all dead code 'design_level' references/overrides #27486
Conversation
9be074c to
f8fb696
Compare
As of the completed migration to ipgen, we no longer make use of the '--flag=fileset_X' selectors to switch out filesets between ip-level and top-level. (e.g. `fileset_top`) Hence, this code is now dead, and can be removed. Note. that there are example uses of 'fileset_partner' as a suggestion for how integrators can use this mechanism to swap out filesets. There is nothing stopping downstream users from continuing to swap-out core filesets using this mechanism and passing flags via the 'sv_flist_gen_flags' hjson variable. However, the documentation should be updated to suggest the use of fusesoc virtual cores as a preferred solution to this problem. Signed-off-by: Harry Callahan <[email protected]>
f8fb696 to
f53e6f8
Compare
|
|
||
| sv_flist_gen_flags: ["--flag=fileset_{design_level}", | ||
| "--mapping=lowrisc:prim_generic:all:0.1"] | ||
| sv_flist_gen_flags: ["--mapping=lowrisc:prim_generic:all:0.1"] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We probably ought to move this to a less-generic property name, so people can override mappings more explicitly. Fine for now, though! (especially since there are only mappings here right now)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, maybe something like a fusesoc_mapping_cores list could be used, with the eval_cmd trick...
sv_flist_gen_opts: ["{fusesoc_cores_root_dirs}",
"run",
"{sv_flist_gen_flags}",
"--target=sim",
"--build-root={build_dir}",
"--setup,
# Something like this...
"{eval_cmd} echo \"{fusesoc_mapping_cores}\" | sed -E 's_(\\S+)_\"--mapping=\\1\"_g",
{fusesoc_core}"]
| fusesoc_cores_root_dirs: ["--cores-root {proj_root}/hw"] | ||
| sv_flist_gen_dir: "{build_dir}/fusesoc-work" | ||
| sv_flist: "{sv_flist_gen_dir}/{fusesoc_core_}.scr" | ||
| sv_flist_gen_flags: ["--flag=fileset_{design_level}"] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A curiosity: How should we document that this property exists, that it is an array of strings, and that it should be used to pass arguments to fusesoc run?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm personally leaning towards leading by example here. Some pages documenting how this can be done, with example .hjson and accompanying comments. Maybe in the future a proper schema for dvsim .hjson would be a useful addition as well, but it is probably a bit too complex to decipher without a worked example or reference pointing to existing code.
#23555 (comment) I'd suggest that we remove the 'fileset_partner' examples once the documentation for how integrators can make use of virtual cores has been written, and notice / opportunity has been given for partners and integrators to migrate.
Related #25747 #27477
Related #23555