Upstreaming verfic file

[dhvll]

Hi @akashlevy

I’m working on upstreaming FEATURE: verific file, currently raising PR for -optimization feature from the
Silimate Yosys fork.

Can you take a look at the PR #5578 and suggest any changes or how should I proceed.

Thanks

[akashlevy]

Your #ifdef indentation isn't correct. #ifdefs typically have 0 indentation in the Yosys codebase. Generally, make sure you are following their conventions.

[akashlevy]

I would also remove the commented out code.

[akashlevy]

Also the #ifdef on line 3713 can probably be added to the #ifdef starting on line 3720, so you don't need to create a new #ifdef.

That's it, otherwise looks ok.

[dhvll]

I have implemented above changes.

I have few questions.

1. For upstreaming multiple Verific options (e.g. -set_ignore_translate_off, -set_relaxed_checking, -set_relaxed_file_ext_modes, -set_relaxed_file_libext_modes, -ignore_module, -set_vhdl_default_library_path.), would you prefer that I open separate upstream issues / PRs per feature, or is it okay to send multiple small patches without individual RFC issues, assuming the changes are minimal and well-scoped?
2. On the implementation side, is the expectation that each feature should be upstreamed as an isolated change/branch, or is it acceptable to incrementally extend the existing #ifdef VERIFIC_SYSTEMVERILOG_SUPPORT option-parsing block, as done in the fork, as long as each PR is logically separated?

[akashlevy]

1. Yes it's fine to send multiple, as long as the upstream maintainers are ok with it. It does cause more burden to review larger changes than smaller changes though, so keep that in mind.
2. Again, this is a matter of preference. I tend to prefer the change that minimizes number of lines changed, to ease the burden for the reviewer.

[dhvll]

I’ve created a pull request for the Verific features. Could you please take a look and let me know if any changes are needed? @akashlevy

[akashlevy]

Many issues... firstly, the #ifdef issue I mentioned is not resolved properly. See how other #ifdefs in the verific.cc source file are done. They are fully outdented.

2. Your ignore module PR is for the VHDL library search path

3. Your relaxed checking PR sets to 0 not 1

4. Your ignore translate off sets to 0 not 1

Honestly, this is a high number of mistakes... please be more diligent when making these PRs so as not to burden the YosysHQ reviewers and myself. Double check your work always.

[dhvll]

Apologies for mistakes. I have fix all the issues above mentioned. Could you please take a look ?

I have open PRs for verific-no_split_complex_ports, verific-ignore_module, verific -set_vhdl_default_library_path