search

chipsalliance / rocket-chip

Rocket Chip Generator
Other
3.15k stars 1.11k forks source link

RFC: Deprecate RegField without a RegFieldDesc? #2283

Open mwachs5 opened 4 years ago

mwachs5 commented 4 years ago

Type of issue: other enhancement

Impact: API modification

Development Phase: proposal

Other information

If the current behavior is a bug, please provide the steps to reproduce the problem:

What is the current behavior?

Currently, when using the regmapper library, one can implement a RegField without any description. The associated RegFieldDesc can be used to automatically generate downstream information like documentation, header files, other formats such as SVD/IP-XACT, etc. But only if people add the descriptions.

What is the expected behavior?

The proposal is to deprecate the RegField functions that use the default of None for the RegFieldDesc. So that people get deprecation warnings if they are implementing regmappers without RegFieldDesc included.

Please tell us about your environment:

What is the use case for changing the behavior?

Requiring designers to auto-document their design in-the-code, vs chasing them after the fact.

richardxia commented 4 years ago

My gut feeling is that mechanically enforcing this may not necessarily have the intended effect. My experience with forcing the presence of metadata at the type level is 1) viewed as burdensome is a statically typed language like Scala, where your code will not be able to compile even if you're trying trying something out and 2) people will just fill in dummy, but type-conforming values (e.g. the empty string or "foo" for any "non-optional" String field).

I'd rather fix this problem at the code review cultural level, where it should be expected that there is discussion and review of not just the HDL but also the metadata.

Also, given that this is a public API out in the open source, it feels a bit too restrictive to force everyone else to conform to SiFive conventions. Although enforcing the correct description of this metadata facilitates SiFive's ability to ship quality, commercial products, we should not push this onto anyone who may be using these APIs for hobby projects or experimental, research projects.

terpstra commented 4 years ago

I was flagged as a reviewer, but have no opinion.

aswaterman commented 4 years ago

I lean towards @richardxia's thinking, but not too strongly.