Add verific -set interface for Yosys-Verific settings#5619
Add verific -set interface for Yosys-Verific settings#5619dhvll wants to merge 4 commits intoYosysHQ:mainfrom
Conversation
Introduce a new structure for managing Yosys-Verific settings, allowing users to get, set, and reset options related to Verific integration. Implement options for ignoring translate pragmas and specifying file extensions for SystemVerilog. Update command-line interface to support these settings, including help documentation for usage examples.
mmicko
left a comment
mmicko
left a comment
There was a problem hiding this comment.
Have to think if there is better way to store current/default data in general, but let's clean it first according to comments
|
hey @mmicko any update related to PR? |
|
@KrystalDelusion Would appreciate another pair of eyes, also on user/docs perspective not just code part. |
KrystalDelusion
left a comment
KrystalDelusion
left a comment
There was a problem hiding this comment.
I would like the question about RemoveFileExt to be addressed before merging. The rest are minor/non-blocking.
| goto check_error; | ||
| } | ||
|
|
||
| if (argidx < GetSize(args) && args[argidx] == "-set") |
There was a problem hiding this comment.
Being able to call verific -set <opt1> <val1> -set <opt2> <val2> might be a nice addition, but current behavior is consistent with verific -cfg so I don't think it's a requirement here.
frontends/verific/verific.cc
Outdated
| case YosysVerificSettings::Type::BOOL: | ||
| log(" %s = %s (bool, default: %s)\n", name.c_str(), | ||
| opt.get<bool>() ? "true" : "false", | ||
| opt.get_default<bool>() ? "true" : "false"); |
There was a problem hiding this comment.
Inconsistent with verific -cfg, but I think the inclusion of a default is an improvement.
frontends/verific/verific.cc
Outdated
| break; | ||
| case YosysVerificSettings::Type::STRING: | ||
| log("verific -set %s \"%s\"\n", name.c_str(), | ||
| opt.get_default<std::string>().c_str()); |
There was a problem hiding this comment.
I think this one is incorrect (it should be opt.get instead), but we don't have any Type::STRING options yet that would show it.
frontends/verific/verific.cc
Outdated
| // Remove default .v extension and add user-specified ones | ||
| veri_file::RemoveFileExt(".v"); |
There was a problem hiding this comment.
Why do we remove ".v" here before adding the user ones? There is also a problem here where if we set (and apply) the setting, and then set it again we don't remove the previous extensions (nor does resetting it), requiring a full restart of Yosys to be able to remove an extension from being parsed as SystemVerilog.
Enhance YosysVerificSettings by introducing a vector to track applied SystemVerilog file extensions, ensuring stale mappings are removed before adding new extensions. Implement an apply_all method to apply all settings to Verific APIs after a reset, improving settings management and consistency.
dhvll
force-pushed
the
verific-set-interface
branch
from
7205c3b to
6435b4a
Compare
3 participants
This PR introduces a verific -set command interface for managing Yosys-specific Verific options, similar to the existing verific -cfg for Verific runtime flags.
Implemented Features :
Usage Examples :
verific -set ignore_translate_off true
verific -set vlog_file_extensions ".v,.sv,.vh,.svh,.h,.inc"
ref: https://yosyshq.discourse.group/t/rfc-upstreaming-a-small-set-of-verific-frontend-options-from-silimate-fork/113