YosysHQ / yosys

Yosys Open SYnthesis Suite
https://yosyshq.net/yosys/
ISC License
3.42k stars 872 forks source link

cleanup: extra_args and argidx #4311

Closed widlarizer closed 2 months ago

widlarizer commented 6 months ago

This PR simplifies and unifies boilerplate by removing unused iteration over pass args which have been left in as placeholders. It also removes commented out arg checks that I think aren't relevant to the pass at hand and have only been copied from another pass.

Incidentally, by setting argidx correctly it also prevents the warning opt_demorgan would unconditionally throw: Warning: Selection "opt_demorgan" did not match any module.

povik commented 6 months ago

I am not sure removing the argument iteration boilerplate is desirable, I am slightly inclined against it, to be frank. The command-line interface to those passes could get expanded at any point.

whitequark commented 6 months ago

Incidentally, by setting argidx correctly it also prevents the warning opt_demorgan would unconditionally throw: Warning: Selection "opt_demorgan" did not match any module.

This seems like a bug fix that should be submitted separately.

widlarizer commented 6 months ago

@whitequark true, opened #4313

@povik It can be expanded at any point by copying from another pass. Or we can make an example pass to make contributing new passes easier. It doesn't seem necessary at all to have this boilerplate in some passes and not others, without explicit TODOs about what should be added. I mean it's a cosmetic change, but I think letting unused code and variables hang around can be distracting

povik commented 6 months ago

Right, it's a cosmetic thing and a matter of taste. Let's leave it up to the dev JF, and if my position will be in the minority let's merge it.

widlarizer commented 2 months ago

Eh, I think we ultimately have bigger fries to fish